diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..1afe48827 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,235 @@ +# AGENTS.md — Soft Serve + +> AI agent guide for [Soft Serve](https://github.com/charmbracelet/soft-serve), a self-hosted Git server with SSH, HTTP, and native git protocol support, plus a built-in TUI. + +## Project Identity + +- **Module:** `github.com/charmbracelet/soft-serve` +- **Language:** Go 1.25+ +- **Binary:** `soft` (entry point: `cmd/soft/main.go`) +- **Database:** SQLite (default) or PostgreSQL, via `jmoiron/sqlx` +- **CLI framework:** `spf13/cobra` (used for both the main CLI and SSH subcommands) +- **SSH framework:** `charm.land/wish/v2` +- **TUI framework:** `charm.land/bubbletea/v2` + `lipgloss/v2` + `glamour/v2` +- **HTTP router:** `gorilla/mux` +- **Config:** YAML file (`$SOFT_SERVE_DATA_PATH/config.yaml`) + env vars (`SOFT_SERVE_*` prefix) + +## Architecture Overview + +Soft Serve runs **four concurrent servers** via `errgroup`: + +| Server | Package | Protocol | Purpose | +|--------|---------|----------|---------| +| SSH | `pkg/ssh/` | SSH | Git push/pull, interactive TUI, admin CLI commands | +| HTTP | `pkg/web/` | HTTP/HTTPS | Git smart HTTP, LFS API, go-get meta, health check | +| Git Daemon | `pkg/daemon/` | Native git (TCP) | Read-only anonymous git access | +| Stats | `pkg/stats/` | HTTP | Prometheus `/metrics` endpoint | + +### Layered Design + +``` +cmd/soft/ → CLI entry points (serve, browse, admin, hook) +pkg/ssh/ → SSH server + middleware + command dispatch +pkg/web/ → HTTP server + middleware + routes +pkg/daemon/ → Git daemon (raw TCP, pktline) +pkg/backend/ → Central business logic orchestrator +pkg/store/ → Data store interface (7 sub-interfaces) +pkg/store/database/ → SQL implementation of Store +pkg/db/ → Database layer (open, migrate, models) +pkg/proto/ → Core domain interfaces (User, Repository) +pkg/access/ → Access level enum (NoAccess → Admin) +pkg/git/ → Git service handlers (upload-pack, receive-pack) +pkg/config/ → Configuration parsing (YAML + env) +pkg/ui/ → TUI components (Bubble Tea models) +``` + +### Canonical Data Flow + +``` +Transport layer (SSH/HTTP/daemon) + → Middleware (context injection, auth, logging) + → Command/Route dispatch + → Backend (business logic) + → Store (data access) + → Database (SQLite/PostgreSQL) +``` + +## Directory Structure + +``` +cmd/ + soft/main.go — Binary entry point + soft/serve/ — `soft serve` command (starts all servers) + soft/admin/ — `soft admin` SSH admin commands + soft/browse/ — `soft browse` TUI browser + soft/hook/ — `soft hook` git hook handler + cmd.go — Shared InitBackendContext / CloseDBContext helpers +git/ — Low-level git operations (repo, commit, tree, tag, refs) +pkg/ + access/ — AccessLevel enum (NoAccess, ReadOnly, ReadWrite, Admin) + backend/ — Central Backend struct: users, repos, auth, LFS, webhooks, cache + config/ — Config struct (YAML + env, SOFT_SERVE_* prefix) + cron/ — Cron scheduler wrapper (robfig/cron) + daemon/ — Git daemon server (TCP, native git protocol) + db/ — Database abstraction (open, migrations, models) + migrate/ — SQL migration files (SQLite + PostgreSQL variants) + models/ — DB model structs + git/ — Git service handlers (upload-pack, receive-pack, LFS transfer) + hooks/ — Git hook generation and interface + jobs/ — Cron job definitions (mirror pull) + jwk/ — JSON Web Key support + lfs/ — Git LFS protocol (client, pointers, scanner, transfers) + log/ — Logger setup + proto/ — Core interfaces (Repository, User, AccessToken, errors) + ssh/ — SSH server, middleware, session handler + cmd/ — 27 SSH subcommands (git, repo, user, token, webhook, settings) + sshutils/ — SSH utility functions + ssrf/ — SSRF protection for outbound requests + stats/ — Prometheus metrics server + storage/ — Storage abstraction (file storage for LFS) + store/ — Store interface (composite of 7 sub-interfaces) + database/ — SQL-backed Store implementation + sync/ — Sync utilities + task/ — Async task manager + ui/ — Bubble Tea TUI components + common/ — Shared TUI state/styles + components/ — Reusable widgets (code, footer, header, selector, statusbar, tabs, viewport) + pages/ — TUI pages (repo list, repo detail) + styles/ — TUI styling + utils/ — General utilities (SanitizeRepo, ValidateRepo) + version/ — Version info + web/ — HTTP server (git smart HTTP, LFS API, auth, health) + webhook/ — Webhook event system (push, branch/tag, repo, collaborator) +testscript/ — Integration tests (testscript framework) +migrations/ — Goose SQL migrations +``` + +## Key Patterns + +### Context-Based Dependency Injection + +All major dependencies are threaded through `context.Context` with typed keys: + +```go +// Injecting +ctx = config.WithContext(ctx, cfg) +ctx = backend.WithContext(ctx, be) + +// Extracting +cfg := config.FromContext(ctx) +be := backend.FromContext(ctx) +``` + +Shared bootstrap in `cmd/cmd.go:InitBackendContext` (used as Cobra `PersistentPreRunE`). + +### Backend as Central Orchestrator + +`pkg/backend/backend.go:Backend` is the single entry point for all business logic. It wraps the Store and provides methods for repos, users, auth, LFS, webhooks, settings, and caching. Handlers and commands call Backend methods — never the Store directly. + +### Store Interface (Composite) + +`pkg/store/store.go:Store` aggregates 7 sub-interfaces: + +- `RepositoryStore` — repo CRUD +- `UserStore` — user CRUD +- `CollaboratorStore` — repo collaborators +- `SettingStore` — server settings +- `LFSStore` — LFS objects/locks +- `AccessTokenStore` — API tokens +- `WebhookStore` — webhook management + +Single SQL implementation in `pkg/store/database/`. + +### Authentication + +**SSH:** Public key verification → `be.UserByPublicKey()`. Keyboard-interactive as keyless fallback. + +**HTTP:** Three schemes via `Authorization` header: +- `Basic` — username/password (bcrypt) or username/access-token +- `Token` — access token directly (`ss_` prefixed, SHA256 hashed) +- `Bearer` — JWT (Ed25519 signed, scoped to repo) + +### Authorization + +Four levels in `pkg/access/access.go`: +- `NoAccess` (0) — denied +- `ReadOnlyAccess` (1) — clone/fetch +- `ReadWriteAccess` (2) — push, LFS locks +- `AdminAccess` (3) — server management + +### SSH Command Dispatch + +Non-PTY SSH sessions go through `CommandMiddleware` which builds a Cobra command tree from `pkg/ssh/cmd/`. The same `cobra.Command` pattern used for the main CLI is reused for SSH commands. + +PTY sessions launch the Bubble Tea TUI via `SessionHandler`. + +### Git Operations + +Git push/pull is handled by shelling out to the `git` binary (not pure Go) via `pkg/git/service.go:gitServiceHandler`. Environment variables pass context (repo name, username, public key) to git hooks. + +### Webhook System + +Events defined in `pkg/webhook/`: push, branch create/delete, tag create/delete, repository create/delete, collaborator add/remove. Delivery via HTTP POST with HMAC-SHA256 signatures. SSRF protection in `pkg/ssrf/`. + +## Build & Test + +```bash +# Build +go build ./cmd/soft + +# Run all tests +go test ./... + +# Run integration tests +go test ./testscript/... + +# Run specific test +go test -run TestName ./pkg/backend/... +``` + +### Test Framework + +- Unit tests: standard Go `testing` + `matryer/is` for assertions +- Integration tests: `rogpeppe/go-internal/testscript` (script-driven, in `testscript/`) +- Test helpers in `pkg/test/` + +## Database + +- **Drivers:** SQLite (`modernc.org/sqlite`, pure Go) and PostgreSQL (`lib/pq`) +- **Access:** `jmoiron/sqlx` with raw SQL queries, `$1`/`$2` placeholders +- **Migrations:** Go-embedded SQL files in `pkg/db/migrate/`, separate `.up.sql`/`.down.sql` per driver +- **Models:** `pkg/db/models/` +- **Transactions:** `db.TransactionContext` helper + +## Configuration + +Loaded from `$SOFT_SERVE_DATA_PATH/config.yaml` with env var overrides (`SOFT_SERVE_*` prefix). Parsed via `caarlos0/env`. Key config sections: + +- `SSH` — listen addr, max timeout, key path +- `HTTP` — listen addr, TLS cert/key, public URL +- `Git` — listen addr, max connections, max timeout +- `Stats` — listen addr +- `LFS` — enabled flag +- `DB` — driver (sqlite/postgres), data source + +## Metrics + +Prometheus metrics throughout the codebase via `promauto`. Exposed at Stats server `/metrics`. Covers SSH connections, HTTP requests, git operations, TUI sessions, LFS transfers. + +## Known TODOs + +- `pkg/backend/backend.go` — proper caching interface (currently basic in-memory) +- `pkg/backend/lfs.go` — S3 storage support for LFS +- `pkg/lfs/ssh_client.go` — Git LFS SSH client (placeholder) +- `pkg/backend/hooks.go` — async hook execution +- `pkg/backend/user.go` — user repository ownership + +## Common Pitfalls + +1. **Never edit `*_templ.go` files** — they are generated. Edit `.templ` sources only. +2. **Context is king** — all dependencies flow through `context.Context`. Use `FromContext` accessors. +3. **Backend, not Store** — handlers call `Backend` methods, never `Store` directly. +4. **Git binary, not go-git** — push/pull shells out to `git`. `go-git` is used for read operations (log, tree, diff). +5. **Dual DB support** — SQL must work on both SQLite and PostgreSQL. Test with both if modifying queries. +6. **SSH commands are Cobra commands** — same pattern as CLI, dispatched in `CommandMiddleware`. +7. **Access levels gate everything** — check `AccessLevel` before any repo operation. diff --git a/cmd/soft/hook/hook.go b/cmd/soft/hook/hook.go index a16cad6d6..aedcc1d61 100644 --- a/cmd/soft/hook/hook.go +++ b/cmd/soft/hook/hook.go @@ -168,5 +168,7 @@ func runCommand(ctx context.Context, in io.Reader, out io.Writer, err io.Writer, cmd.Stdin = in cmd.Stdout = out cmd.Stderr = err + cfg := config.FromContext(ctx) + cmd.Env = cfg.Environ() return cmd.Run() } diff --git a/cmd/soft/serve/serve.go b/cmd/soft/serve/serve.go index 7472f3d0b..e46acbb53 100644 --- a/cmd/soft/serve/serve.go +++ b/cmd/soft/serve/serve.go @@ -12,7 +12,9 @@ import ( "syscall" "time" + "charm.land/log/v2" "github.com/charmbracelet/soft-serve/cmd" + "github.com/charmbracelet/soft-serve/pkg/access" "github.com/charmbracelet/soft-serve/pkg/backend" "github.com/charmbracelet/soft-serve/pkg/config" "github.com/charmbracelet/soft-serve/pkg/db" @@ -70,6 +72,26 @@ var ( return fmt.Errorf("migration error: %w", err) } + // Apply config-driven settings so operators can automate + // server setup without a post-start SSH session. + { + cfgCtx := config.FromContext(ctx) + be := backend.FromContext(ctx) + logger := log.FromContext(ctx).WithPrefix("server") + if cfgCtx.AnonAccess != "" { + if err := be.SetAnonAccess(ctx, access.ParseAccessLevel(cfgCtx.AnonAccess)); err != nil { + return fmt.Errorf("set anon_access: %w", err) + } + logger.Debug("applied anon_access from config", "level", cfgCtx.AnonAccess) + } + if cfgCtx.AllowKeyless != nil { + if err := be.SetAllowKeyless(ctx, *cfgCtx.AllowKeyless); err != nil { + return fmt.Errorf("set allow_keyless: %w", err) + } + logger.Debug("applied allow_keyless from config", "value", *cfgCtx.AllowKeyless) + } + } + s, err := NewServer(ctx) if err != nil { return fmt.Errorf("start server: %w", err) diff --git a/cmd/soft/serve/server.go b/cmd/soft/serve/server.go index fda090050..ce1d9d40c 100644 --- a/cmd/soft/serve/server.go +++ b/cmd/soft/serve/server.go @@ -5,7 +5,10 @@ import ( "crypto/tls" "errors" "fmt" + "net" "net/http" + "sync" + "time" "charm.land/log/v2" @@ -34,8 +37,10 @@ type Server struct { Backend *backend.Backend DB *db.DB - logger *log.Logger - ctx context.Context + logger *log.Logger + ctx context.Context + shutdownOnce sync.Once + shutdownErr error } // NewServer returns a new *Server configured to serve Soft Serve. The SSH @@ -59,7 +64,11 @@ func NewServer(ctx context.Context) (*Server, error) { // Add cron jobs. sched := cron.NewScheduler(ctx) for n, j := range jobs.List() { - id, err := sched.AddFunc(j.Runner.Spec(ctx), j.Runner.Func(ctx)) + spec := j.Runner.Spec(ctx) + if spec == "" { + continue + } + id, err := sched.AddFunc(spec, j.Runner.Func(ctx)) if err != nil { logger.Warn("error adding cron job", "job", n, "err", err) } @@ -111,48 +120,131 @@ func (s *Server) ReloadCertificates() error { return s.CertLoader.Reload() } -// Start starts the SSH server. +// Start starts all configured servers. func (s *Server) Start() error { - errg, _ := errgroup.WithContext(s.ctx) + // Pre-bind all configured listeners synchronously. If any bind fails + // (e.g. EACCES on a privileged port or address already in use) we + // return an error immediately with a clear message, rather than + // silently continuing while other servers run without the failing one. + var ( + sshLn net.Listener + gitLn net.Listener + httpLn net.Listener + statsLn net.Listener + err error + ) + + // closeAll holds listeners opened so far; cleared once all binds + // succeed so that the defer does not double-close them. + var closeAll []net.Listener + defer func() { + for _, ln := range closeAll { + ln.Close() //nolint:errcheck + } + }() + + if s.Config.SSH.Enabled { + sshLn, err = net.Listen("tcp", s.Config.SSH.ListenAddr) + if err != nil { + return fmt.Errorf("ssh listen %s: %w", s.Config.SSH.ListenAddr, err) + } + closeAll = append(closeAll, sshLn) + } + + if s.Config.Git.Enabled { + gitLn, err = net.Listen("tcp", s.Config.Git.ListenAddr) + if err != nil { + return fmt.Errorf("git daemon listen %s: %w", s.Config.Git.ListenAddr, err) + } + closeAll = append(closeAll, gitLn) + } + + if s.Config.HTTP.Enabled { + httpLn, err = net.Listen("tcp", s.Config.HTTP.ListenAddr) + if err != nil { + return fmt.Errorf("http listen %s: %w", s.Config.HTTP.ListenAddr, err) + } + closeAll = append(closeAll, httpLn) + } + + if s.Config.Stats.Enabled { + statsLn, err = net.Listen("tcp", s.Config.Stats.ListenAddr) + if err != nil { + return fmt.Errorf("stats listen %s: %w", s.Config.Stats.ListenAddr, err) + } + closeAll = append(closeAll, statsLn) + } + + // All binds succeeded; goroutines take ownership of each listener. + closeAll = nil + + errg, gctx := errgroup.WithContext(s.ctx) + + // External monitor goroutine: if any server goroutine returns an + // error, shut down all remaining servers so errg.Wait() unblocks + // and the caller sees the error. Runs *outside* the errgroup so it + // never blocks errg.Wait() itself. + // gctx is always cancelled — either by an erroring goroutine or by + // errg.Wait() itself on return — so this goroutine never leaks. + go func() { + <-gctx.Done() + // errgroup (golang.org/x/sync >= v0.12.0) uses + // context.WithCancelCause internally and calls cancel(g.err) + // on Wait(). If every goroutine returns nil, g.err == nil and + // context.Cause(gctx) == nil — skip to avoid a redundant call. + // If a goroutine returned a non-nil error, Cause != nil and we + // must shut down the remaining servers ourselves. + // + // Invariant: all ErrServerClosed variants MUST be filtered in + // the errg.Go wrappers above; any that leak would incorrectly + // trigger this path. + // + // s.Shutdown is idempotent (shutdownOnce-protected), so it is + // safe if serve.go's SIGTERM handler races to call it first. + if context.Cause(gctx) == nil { + return + } + shutCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if shutErr := s.Shutdown(shutCtx); shutErr != nil { + s.logger.Error("error shutting down after unexpected server failure", "err", shutErr) + } + }() - // optionally start the SSH server if s.Config.SSH.Enabled { errg.Go(func() error { s.logger.Print("Starting SSH server", "addr", s.Config.SSH.ListenAddr) - if err := s.SSHServer.ListenAndServe(); !errors.Is(err, ssh.ErrServerClosed) { + if err := s.SSHServer.Serve(sshLn); !errors.Is(err, ssh.ErrServerClosed) { return err } return nil }) } - // optionally start the git daemon if s.Config.Git.Enabled { errg.Go(func() error { s.logger.Print("Starting Git daemon", "addr", s.Config.Git.ListenAddr) - if err := s.GitDaemon.ListenAndServe(); !errors.Is(err, daemon.ErrServerClosed) { + if err := s.GitDaemon.Serve(gitLn); !errors.Is(err, daemon.ErrServerClosed) { return err } return nil }) } - // optionally start the HTTP server if s.Config.HTTP.Enabled { errg.Go(func() error { s.logger.Print("Starting HTTP server", "addr", s.Config.HTTP.ListenAddr) - if err := s.HTTPServer.ListenAndServe(); !errors.Is(err, http.ErrServerClosed) { + if err := s.HTTPServer.Serve(httpLn); !errors.Is(err, http.ErrServerClosed) { return err } return nil }) } - // optionally start the Stats server if s.Config.Stats.Enabled { errg.Go(func() error { s.logger.Print("Starting Stats server", "addr", s.Config.Stats.ListenAddr) - if err := s.StatsServer.ListenAndServe(); !errors.Is(err, http.ErrServerClosed) { + if err := s.StatsServer.Serve(statsLn); !errors.Is(err, http.ErrServerClosed) { return err } return nil @@ -166,8 +258,19 @@ func (s *Server) Start() error { return errg.Wait() } -// Shutdown lets the server gracefully shutdown. +// Shutdown lets the server gracefully shutdown. It is safe to call +// concurrently; only the first call performs the actual shutdown. +// Subsequent callers block until the first completes and then receive +// the same error, so the process exit code reflects the real outcome +// regardless of which path (monitor goroutine or SIGTERM handler) wins. func (s *Server) Shutdown(ctx context.Context) error { + s.shutdownOnce.Do(func() { + s.shutdownErr = s.shutdown(ctx) + }) + return s.shutdownErr +} + +func (s *Server) shutdown(ctx context.Context) error { errg, ctx := errgroup.WithContext(ctx) errg.Go(func() error { return s.GitDaemon.Shutdown(ctx) diff --git a/git/commit.go b/git/commit.go index e833c8a7f..42aad514f 100644 --- a/git/commit.go +++ b/git/commit.go @@ -9,10 +9,15 @@ import ( // ZeroID is the zero hash. const ZeroID = git.EmptyID +// zeroHashPattern matches an all-zero SHA-1 (40 hex zeros) or SHA-256 +// (64 hex zeros) object ID. The alternation prevents matching lengths that +// are not valid hash sizes (e.g. 50 zeros). Compiled once at package init to +// avoid repeated allocations in hot paths such as webhook delivery. +var zeroHashPattern = regexp.MustCompile(`^(0{40}|0{64})$`) + // IsZeroHash returns whether the hash is a zero hash. func IsZeroHash(h string) bool { - pattern := regexp.MustCompile(`^0{40,}$`) - return pattern.MatchString(h) + return zeroHashPattern.MatchString(h) } // Commit is a wrapper around git.Commit with helper methods. @@ -28,6 +33,9 @@ func (cl Commits) Len() int { return len(cl) } func (cl Commits) Swap(i, j int) { cl[i], cl[j] = cl[j], cl[i] } // Less implements sort.Interface. +// Sorts by author date (not committer date) so the displayed order matches +// the original authorship timeline. Cherry-picks and rebases will therefore +// appear at the position of their original authoring, not the rebase time. func (cl Commits) Less(i, j int) bool { return cl[i].Author.When.After(cl[j].Author.When) } diff --git a/git/diff.go b/git/diff.go new file mode 100644 index 000000000..58e97b1f5 --- /dev/null +++ b/git/diff.go @@ -0,0 +1,33 @@ +package git + +import "strings" + +const maxDiffSize = 512 * 1024 // 512 KB + +// DiffRefs returns the unified diff between two git refs (branches, tags, commits). +// Returns empty string if refs are identical or if either ref is empty. +// Output is capped at 512 KB to prevent unbounded memory use. +func (r *Repository) DiffRefs(from, to string) (string, error) { + if from == "" || to == "" { + return "", nil + } + // Use git diff --no-color from..to to get unified diff between two refs + out, err := NewCommand("diff", "--no-color", from+".."+to).RunInDir(r.Path) + if err != nil { + // git diff exits non-zero only on errors (e.g. invalid ref, not a git repo). + // Non-zero exit with output means partial results; return them. + if len(out) > 0 { + return truncateDiff(string(out)), nil + } + return "", err + } + return truncateDiff(string(out)), nil +} + +// truncateDiff trims trailing newlines and caps the output at maxDiffSize. +func truncateDiff(s string) string { + if len(s) > maxDiffSize { + return s[:maxDiffSize] + "\n\n[diff truncated — output exceeded 512 KB]" + } + return strings.TrimRight(s, "\n") +} diff --git a/git/utils.go b/git/utils.go index b4ca50fc6..5e1e72559 100644 --- a/git/utils.go +++ b/git/utils.go @@ -2,6 +2,7 @@ package git import ( "os" + "path" "path/filepath" "github.com/gobwas/glob" @@ -10,7 +11,7 @@ import ( // LatestFile returns the contents of the first file at the specified path pattern in the repository and its file path. func LatestFile(repo *Repository, ref *Reference, pattern string) (string, string, error) { g := glob.MustCompile(pattern) - dir := filepath.Dir(pattern) + dir := path.Dir(pattern) if ref == nil { head, err := repo.HEAD() if err != nil { @@ -28,7 +29,7 @@ func LatestFile(repo *Repository, ref *Reference, pattern string) (string, strin } for _, e := range ents { te := e - fp := filepath.Join(dir, te.Name()) + fp := path.Join(dir, te.Name()) if te.IsTree() { continue } diff --git a/go.mod b/go.mod index 510acd189..494c023fc 100644 --- a/go.mod +++ b/go.mod @@ -95,6 +95,7 @@ require ( golang.org/x/net v0.51.0 // indirect golang.org/x/sys v0.42.0 // indirect golang.org/x/text v0.35.0 // indirect + golang.org/x/time v0.15.0 // indirect golang.org/x/tools v0.42.0 // indirect google.golang.org/protobuf v1.36.8 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect diff --git a/go.sum b/go.sum index f9d3e2d65..6fc9450a3 100644 --- a/go.sum +++ b/go.sum @@ -275,6 +275,8 @@ golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= +golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= +golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/pkg/backend/access_token.go b/pkg/backend/access_token.go index b8db671a1..d8da53bf1 100644 --- a/pkg/backend/access_token.go +++ b/pkg/backend/access_token.go @@ -12,7 +12,13 @@ import ( // CreateAccessToken creates an access token for user. func (b *Backend) CreateAccessToken(ctx context.Context, user proto.User, name string, expiresAt time.Time) (string, error) { - token := GenerateToken() + token, err := GenerateToken() + if err != nil { + return "", err + } + if token == "" { + return "", errors.New("generated token is empty") + } tokenHash := HashToken(token) name = utils.Sanitize(name) @@ -33,11 +39,6 @@ func (b *Backend) CreateAccessToken(ctx context.Context, user proto.User, name s // DeleteAccessToken deletes an access token for a user. func (b *Backend) DeleteAccessToken(ctx context.Context, user proto.User, id int64) error { err := b.db.TransactionContext(ctx, func(tx *db.Tx) error { - _, err := b.store.GetAccessToken(ctx, tx, id) - if err != nil { - return db.WrapError(err) - } - if err := b.store.DeleteAccessTokenForUser(ctx, tx, user.ID(), id); err != nil { return db.WrapError(err) } diff --git a/pkg/backend/auth.go b/pkg/backend/auth.go index 5309e410e..8b6fc7c9d 100644 --- a/pkg/backend/auth.go +++ b/pkg/backend/auth.go @@ -4,16 +4,14 @@ import ( "crypto/rand" "crypto/sha256" "encoding/hex" + "fmt" - "charm.land/log/v2" "golang.org/x/crypto/bcrypt" ) -const saltySalt = "salty-soft-serve" - // HashPassword hashes the password using bcrypt. func HashPassword(password string) (string, error) { - crypt, err := bcrypt.GenerateFromPassword([]byte(password+saltySalt), bcrypt.DefaultCost) + crypt, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) if err != nil { return "", err } @@ -23,23 +21,24 @@ func HashPassword(password string) (string, error) { // VerifyPassword verifies the password against the hash. func VerifyPassword(password, hash string) bool { - err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password+saltySalt)) + err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password)) return err == nil } // GenerateToken returns a random unique token. -func GenerateToken() string { +func GenerateToken() (string, error) { buf := make([]byte, 20) if _, err := rand.Read(buf); err != nil { - log.Error("unable to generate access token") - return "" + return "", fmt.Errorf("generate access token: %w", err) } - return "ss_" + hex.EncodeToString(buf) + return "ss_" + hex.EncodeToString(buf), nil } -// HashToken hashes the token using sha256. +// HashToken returns the SHA-256 hex digest of the token. The 20-byte (160-bit) +// random token provides sufficient entropy to make rainbow-table attacks +// impractical. A HMAC-with-server-secret scheme would be more defensive. func HashToken(token string) string { - sum := sha256.Sum256([]byte(token + saltySalt)) + sum := sha256.Sum256([]byte(token)) return hex.EncodeToString(sum[:]) } diff --git a/pkg/backend/auth_test.go b/pkg/backend/auth_test.go index db40db3b9..e7ff5d798 100644 --- a/pkg/backend/auth_test.go +++ b/pkg/backend/auth_test.go @@ -23,14 +23,20 @@ func TestVerifyPassword(t *testing.T) { } func TestGenerateToken(t *testing.T) { - token := GenerateToken() + token, err := GenerateToken() + if err != nil { + t.Fatal(err) + } if token == "" { t.Fatal("token is empty") } } func TestHashToken(t *testing.T) { - token := GenerateToken() + token, err := GenerateToken() + if err != nil { + t.Fatal(err) + } hash := HashToken(token) if hash == "" { t.Fatal("hash is empty") diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go index 95d45ec2d..714f01179 100644 --- a/pkg/backend/backend.go +++ b/pkg/backend/backend.go @@ -8,18 +8,20 @@ import ( "github.com/charmbracelet/soft-serve/pkg/db" "github.com/charmbracelet/soft-serve/pkg/store" "github.com/charmbracelet/soft-serve/pkg/task" + "golang.org/x/sync/singleflight" ) // Backend is the Soft Serve backend that handles users, repositories, and // server settings management and operations. type Backend struct { - ctx context.Context - cfg *config.Config - db *db.DB - store store.Store - logger *log.Logger - cache *cache - manager *task.Manager + ctx context.Context + cfg *config.Config + db *db.DB + store store.Store + logger *log.Logger + cache *cache + manager *task.Manager + reposSFG singleflight.Group // deduplicate concurrent Repositories() calls } // New returns a new Soft Serve backend. diff --git a/pkg/backend/cache.go b/pkg/backend/cache.go index 8d99401dd..af411d4e5 100644 --- a/pkg/backend/cache.go +++ b/pkg/backend/cache.go @@ -30,6 +30,3 @@ func (c *cache) Delete(repo string) { c.repos.Remove(repo) } -func (c *cache) Len() int { - return c.repos.Len() -} diff --git a/pkg/backend/hooks.go b/pkg/backend/hooks.go index f6c644671..f705e1148 100644 --- a/pkg/backend/hooks.go +++ b/pkg/backend/hooks.go @@ -4,22 +4,122 @@ import ( "context" "io" "os" - "sync" + "time" "github.com/charmbracelet/soft-serve/git" "github.com/charmbracelet/soft-serve/pkg/hooks" "github.com/charmbracelet/soft-serve/pkg/proto" "github.com/charmbracelet/soft-serve/pkg/sshutils" "github.com/charmbracelet/soft-serve/pkg/webhook" + "gopkg.in/yaml.v3" ) var _ hooks.Hooks = (*Backend)(nil) -// PostReceive is called by the git post-receive hook. -// -// It implements Hooks. -func (d *Backend) PostReceive(_ context.Context, _ io.Writer, _ io.Writer, repo string, args []hooks.HookArg) { +// repoMetaConfig holds repository metadata fields synced from .soft-serve.yaml. +type repoMetaConfig struct { + Description string `yaml:"description"` + Private *bool `yaml:"private"` + Hidden *bool `yaml:"hidden"` +} + +// PostReceive is called by the git post-receive hook. It implements Hooks. +// Metadata sync (.soft-serve.yaml) is performed asynchronously so the push +// response is not blocked by DB writes or git tree reads. +func (d *Backend) PostReceive(ctx context.Context, _ io.Writer, _ io.Writer, repo string, args []hooks.HookArg) { d.logger.Debug("post-receive hook called", "repo", repo, "args", args) + + // Capture the pushing user before the goroutine is launched so that the + // caller's context (which may be cancelled after the push completes) does + // not need to remain valid inside the goroutine. + user := proto.UserFromContext(ctx) + + // Sync .soft-serve.yaml metadata asynchronously so the push + // response is not blocked by DB writes or git tree reads. + go func() { + // The 30-second timeout covers metadata sync (DB writes, YAML parse). + // PushMirrors is called from syncRepoMeta with d.ctx (the backend root + // context, not syncCtx), so mirror pushes are NOT constrained by this + // timeout — they run with their own per-push mirrorPushTimeout. + syncCtx, cancel := context.WithTimeout(d.ctx, 30*time.Second) + defer cancel() + d.syncRepoMeta(syncCtx, repo, user) + }() +} + +// syncRepoMeta reads .soft-serve.yaml from HEAD and applies non-zero fields +// to the repository backend. Private and hidden require admin access. +// user is the pushing user captured from the caller's context before the +// goroutine was launched. +func (d *Backend) syncRepoMeta(ctx context.Context, repo string, user proto.User) { + r, err := d.Repository(ctx, repo) + if err != nil { + d.logger.Warn("post-receive: failed to find repository", "repo", repo, "err", err) + return + } + + // Run push mirrors with a separate context derived from the backend's root + // context so they are not limited by the 30-second syncCtx. Each mirror + // goroutine inside PushMirrors creates its own per-push timeout via + // mirrorPushTimeout; using the backend root context here lets that inner + // timeout operate at full duration. + d.PushMirrors(d.ctx, r) + + gr, err := r.Open() + if err != nil { + // empty or invalid repo — skip + return + } + + const maxMetaFileSize = 64 * 1024 // 64 KB + + content, _, err := git.LatestFile(gr, nil, ".soft-serve.yaml") + if err != nil { + // file absent or no commits yet — no-op + return + } + if len(content) > maxMetaFileSize { + d.logger.Warnf("post-receive: .soft-serve.yaml exceeds %d bytes, skipping", maxMetaFileSize) + return + } + + var meta repoMetaConfig + if err := yaml.Unmarshal([]byte(content), &meta); err != nil { + d.logger.Warnf("post-receive: parse .soft-serve.yaml: %v", err) + return + } + + // Only admins may change visibility. Use the user captured from the + // caller's context rather than looking it up from d.ctx, which would + // return nil because d.ctx has no user attached. + var isAdmin bool + if user != nil { + isAdmin = user.IsAdmin() + } + + if meta.Description != "" { + const maxDescLen = 2048 + desc := meta.Description + if runes := []rune(desc); len(runes) > maxDescLen { + desc = string(runes[:maxDescLen]) + } + if err := d.SetDescription(ctx, repo, desc); err != nil { + d.logger.Warnf("post-receive: set description: %v", err) + } + } + + if isAdmin { + if meta.Private != nil { + if err := d.SetPrivate(ctx, repo, *meta.Private); err != nil { + d.logger.Warnf("post-receive: set private: %v", err) + } + } + if meta.Hidden != nil { + if err := d.SetHidden(ctx, repo, *meta.Hidden); err != nil { + d.logger.Warnf("post-receive: set hidden: %v", err) + } + } + } } // PreReceive is called by the git pre-receive hook. @@ -35,7 +135,10 @@ func (d *Backend) PreReceive(_ context.Context, _ io.Writer, _ io.Writer, repo s func (d *Backend) Update(ctx context.Context, _ io.Writer, _ io.Writer, repo string, arg hooks.HookArg) { d.logger.Debug("update hook called", "repo", repo, "arg", arg) - // Find user + // Find user from hook environment variables. These are process-global but + // safe because each hook invocation is run in a separate subprocess (see + // pkg/hooks/gen.go); concurrent pushes in the same server process never + // share the hook subprocess's environment. var user proto.User if pubkey := os.Getenv("SOFT_SERVE_PUBLIC_KEY"); pubkey != "" { pk, _, err := sshutils.ParseAuthorizedKey(pubkey) @@ -57,7 +160,7 @@ func (d *Backend) Update(ctx context.Context, _ io.Writer, _ io.Writer, repo str return } } else { - d.logger.Error("error finding user") + d.logger.Warn("error finding user: neither SOFT_SERVE_PUBLIC_KEY nor SOFT_SERVE_USERNAME is set in hook environment") return } @@ -68,8 +171,9 @@ func (d *Backend) Update(ctx context.Context, _ io.Writer, _ io.Writer, repo str return } - // TODO: run this async - // This would probably need something like an RPC server to communicate with the hook process. + // Webhook delivery runs synchronously in the update hook subprocess. + // Async dispatch would require an IPC channel between the hook process + // and the main server; not currently implemented. if git.IsZeroHash(arg.OldSha) || git.IsZeroHash(arg.NewSha) { wh, err := webhook.NewBranchTagEvent(ctx, user, r, arg.RefName, arg.OldSha, arg.NewSha) if err != nil { @@ -92,19 +196,9 @@ func (d *Backend) Update(ctx context.Context, _ io.Writer, _ io.Writer, repo str func (d *Backend) PostUpdate(ctx context.Context, _ io.Writer, _ io.Writer, repo string, args ...string) { d.logger.Debug("post-update hook called", "repo", repo, "args", args) - var wg sync.WaitGroup - - // Populate last-modified file. - wg.Add(1) - go func() { - defer wg.Done() - if err := populateLastModified(ctx, d, repo); err != nil { - d.logger.Error("error populating last-modified", "repo", repo, "err", err) - return - } - }() - - wg.Wait() + if err := populateLastModified(ctx, d, repo); err != nil { + d.logger.Error("error populating last-modified", "repo", repo, "err", err) + } } func populateLastModified(ctx context.Context, d *Backend, name string) error { diff --git a/pkg/backend/lfs.go b/pkg/backend/lfs.go index 16569094a..fa11f9799 100644 --- a/pkg/backend/lfs.go +++ b/pkg/backend/lfs.go @@ -3,7 +3,9 @@ package backend import ( "context" "errors" + "fmt" "io" + "os" "path" "path/filepath" "strconv" @@ -40,17 +42,34 @@ func StoreRepoMissingLFSObjects(ctx context.Context, repo proto.Repository, dbx } defer content.Close() //nolint: errcheck + // Disk-ahead-of-DB invariant: strg.Put writes the object to the + // filesystem; CreateLFSObject writes the DB row. If strg.Put succeeds + // but the DB transaction rolls back, the object file remains on disk + // without a corresponding DB row. The re-registration path below + // (obj != nil && obj.ID == 0) handles this case on the next download + // attempt by re-inserting the DB row without re-downloading the data. + // Write object to disk first (outside transaction to avoid holding + // DB slot for long downloads). If DB insert fails, delete file. + objPath := path.Join("objects", p.RelativePath()) + if _, err := strg.Put(objPath, content); err != nil { + return fmt.Errorf("failed to write LFS object to disk: %w", err) + } return dbx.TransactionContext(ctx, func(tx *db.Tx) error { if err := store.CreateLFSObject(ctx, tx, repo.ID(), p.Oid, p.Size); err != nil { - return db.WrapError(err) + // DB insert failed — clean up on-disk file to avoid orphan. + if rmErr := os.Remove(objPath); rmErr != nil { + return errors.Join(err, rmErr) + } + return nil // os.Remove succeeded, propagate DB error + + return err } - - _, err := strg.Put(path.Join("objects", p.RelativePath()), content) - return err + return nil }) }) } + const lfsBatchSize = 20 var batch []lfs.Pointer for pointer := range pointerChan { obj, err := store.GetLFSObjectByOid(ctx, dbx, repo.ID(), pointer.Oid) @@ -63,23 +82,45 @@ func StoreRepoMissingLFSObjects(ctx context.Context, repo proto.Repository, dbx return err } + if exist && obj.ID != 0 { + // fully synced — skip + continue + } if exist && obj.ID == 0 { + // Disk-ahead-of-DB recovery: object is on disk but not in the DB. + // Validate the pointer before re-registering it to guard against + // malformed OIDs reaching the DB (defense-in-depth; the LFS scanner + // already validates OIDs, but we check again here to be explicit). + if !pointer.IsValid() { + return fmt.Errorf("lfs: invalid pointer during re-registration: oid=%s", pointer.Oid) + } if err := store.CreateLFSObject(ctx, dbx, repo.ID(), pointer.Oid, pointer.Size); err != nil { return db.WrapError(err) } - } else { - batch = append(batch, pointer.Pointer) - // Limit batch requests to 20 objects - if len(batch) >= 20 { - if err := download(batch); err != nil { - return err - } - - batch = nil + continue + } + // not on disk — add to download batch + batch = append(batch, pointer.Pointer) + // Limit batch requests to lfsBatchSize objects + if len(batch) >= lfsBatchSize { + if err := download(batch); err != nil { + return err } + + batch = nil + } + } + + if len(batch) > 0 { + if err := download(batch); err != nil { + return err } } + // errChan is closed by SearchPointerBlobs after wg.Wait() completes. + // If SearchPointerBlobs sent an error before closing, ok is true and err + // holds the error. If it closed without sending (no error), ok is false + // and err is nil — the zero value — which we correctly ignore. if err, ok := <-errChan; ok { return err } diff --git a/pkg/backend/push_mirror.go b/pkg/backend/push_mirror.go new file mode 100644 index 000000000..0e81417f8 --- /dev/null +++ b/pkg/backend/push_mirror.go @@ -0,0 +1,204 @@ +package backend + +import ( + "context" + "fmt" + "net/url" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "time" + + "github.com/charmbracelet/soft-serve/pkg/db" + "github.com/charmbracelet/soft-serve/pkg/db/models" + "github.com/charmbracelet/soft-serve/pkg/proto" + "github.com/charmbracelet/soft-serve/pkg/ssrf" +) + +// AddPushMirror adds a push mirror to a repository. +func (b *Backend) AddPushMirror(ctx context.Context, repo proto.Repository, name, remoteURL string) error { + return b.db.TransactionContext(ctx, func(tx *db.Tx) error { + return b.store.CreatePushMirror(ctx, tx, repo.ID(), name, remoteURL) + }) +} + +// RemovePushMirror removes a push mirror from a repository. +func (b *Backend) RemovePushMirror(ctx context.Context, repo proto.Repository, name string) error { + return b.db.TransactionContext(ctx, func(tx *db.Tx) error { + return b.store.DeletePushMirror(ctx, tx, repo.ID(), name) + }) +} + +// ListPushMirrors lists all push mirrors for a repository. +func (b *Backend) ListPushMirrors(ctx context.Context, repo proto.Repository) ([]models.PushMirror, error) { + var mirrors []models.PushMirror + err := b.db.TransactionContext(ctx, func(tx *db.Tx) error { + var err error + mirrors, err = b.store.GetPushMirrorsByRepoID(ctx, tx, repo.ID()) + return err + }) + return mirrors, err +} + +// PushMirrors triggers an async push to all enabled mirrors for a repository. +// Called from the post-receive hook. Errors are logged but not fatal. +func (b *Backend) PushMirrors(ctx context.Context, repo proto.Repository) { + mirrors, err := b.ListPushMirrors(ctx, repo) + if err != nil { + b.logger.Warn("push-mirror: failed to list mirrors", "repo", repo.Name(), "err", err) + return + } + repoPath := b.repoPath(repo.Name()) + const mirrorPushTimeout = 10 * time.Minute + const maxConcurrentPushes = 5 + sem := make(chan struct{}, maxConcurrentPushes) + var wg sync.WaitGroup + for _, m := range mirrors { + if !m.Enabled { + continue + } + u, err := url.Parse(m.RemoteURL) + if err == nil && (u.Scheme == "http" || u.Scheme == "https") { + if ssrfErr := ssrf.ValidateURL(ctx, m.RemoteURL); ssrfErr != nil { + b.logger.Warn("push mirror: SSRF check failed", "remote", m.RemoteURL, "err", ssrfErr) + continue + } + } else if err == nil && (u.Scheme == "ssh" || u.Scheme == "git+ssh" || u.Scheme == "ssh+git") { + // Validate ssh:// / git+ssh:// / ssh+git:// scheme URLs against SSRF. + // Note: unlike the HTTP client (NewSecureClient), the SSH mirror uses + // the git subprocess which re-resolves the hostname at dial time — + // there is a DNS rebinding window between this check and the actual + // connection. Mitigate by ensuring the host resolves to a public IP. + if ssrfErr := ssrf.ValidateHost(ctx, u.Hostname()); ssrfErr != nil { + b.logger.Warn("push mirror: SSRF check failed", "remote", m.RemoteURL, "err", ssrfErr) + continue + } + } else // SCP-style remote (e.g. git@host:repo) — url.Parse either fails or + // produces an empty scheme. The err != nil branch is for genuine + // parse errors; the u.Scheme == "" branch is for SCP-style strings. + if err != nil || u.Scheme == "" { + // SCP-style remote (e.g. git@host:repo) — url.Parse either fails or + // produces an empty scheme. Both cases require manual host extraction. + // Note: url.Parse rarely errors on SCP-style strings (it typically + // succeeds with an empty scheme), so the err != nil branch here is + // largely dead code in practice — the empty-scheme check is the + // common path. + host, scpErr := extractSCPHost(m.RemoteURL) + if scpErr != nil { + b.logger.Warn("push mirror: cannot extract host from SCP-style remote", "remote", m.RemoteURL, "err", scpErr) + continue + } + if ssrfErr := ssrf.ValidateHost(ctx, host); ssrfErr != nil { + b.logger.Warn("push mirror: SSRF check failed", "remote", m.RemoteURL, "err", ssrfErr) + continue + } + } else { + // Block git://, file://, and any other unrecognized scheme. + // file:// could allow access to local filesystem paths and is + // blocked here even though it does not carry a network host. + schemeErr := fmt.Errorf("push mirror: unsupported URL scheme %q", u.Scheme) + b.logger.Warn(schemeErr.Error(), "remote", m.RemoteURL) + continue + } + sem <- struct{}{} // acquire + wg.Add(1) + go func(m models.PushMirror) { + defer wg.Done() + defer func() { <-sem }() // release + mirrorCtx, cancel := context.WithTimeout(ctx, mirrorPushTimeout) + defer cancel() + // m.RemoteURL is passed as a positional argument to exec.Command (not shell-expanded), + // so there is no shell injection risk. SSRF validation has already run above. + cmd := exec.CommandContext(mirrorCtx, "git", "push", "--mirror", m.RemoteURL) + cmd.Dir = repoPath + // Note: HOME and PATH are sourced from the current process environment. + // If soft-serve runs as a system service with a restricted environment, + // these may be empty; in that case, git push may fail. Ensure the service + // environment includes a valid PATH (e.g., /usr/bin:/usr/local/bin). + cmd.Env = []string{ + "HOME=" + os.Getenv("HOME"), + "PATH=" + os.Getenv("PATH"), + // Prevent git from loading system-wide or user-level config files. + // An operator-controlled HOME could otherwise redirect git to an + // attacker-supplied .gitconfig. These variables are supported by + // git 2.32+ (GIT_CONFIG_COUNT) and older (GIT_CONFIG_NOSYSTEM). + "GIT_CONFIG_NOSYSTEM=1", + "GIT_CONFIG_COUNT=0", + } + if sshCmd := os.Getenv("GIT_SSH_COMMAND"); sshCmd != "" { + // Warn if operator sets GIT_SSH_COMMAND but no pinned known_hosts entry + // exists for the target host — the default SSH setup pins fingerprints, + // but an operator override bypasses this mitigation. + knownHostsFile := filepath.Join(b.cfg.DataPath, "mirror_known_hosts") + if _, err := os.Stat(knownHostsFile); err != nil && os.IsNotExist(err) { + b.logger.Warn("push-mirror: GIT_SSH_COMMAND set but no known_hosts file exists — DNS rebinding mitigation bypassed", "remote", m.RemoteURL) + } + // Apply the same safety check used for SSH_AUTH_SOCK: a newline + // or NUL in an env var value would malform the env block passed + // to the subprocess. + if strings.ContainsAny(sshCmd, "\n\r\x00") { + b.logger.Warn("push-mirror: GIT_SSH_COMMAND contains unsafe characters, ignoring operator override") + } else { + cmd.Env = append(cmd.Env, "GIT_SSH_COMMAND="+sshCmd) + } + } else { + // Build a default GIT_SSH_COMMAND that pins host fingerprints into a + // per-server known_hosts file. StrictHostKeyChecking=accept-new records + // the fingerprint on first connection and rejects changes thereafter, + // narrowing the DNS rebinding window: an attacker who swaps the DNS + // record after the initial SSRF check will be blocked by the pinned key. + // This is a best-effort mitigation — it does not eliminate the window + // between SSRF validation and the first SSH handshake. + knownHostsFile := filepath.Join(b.cfg.DataPath, "mirror_known_hosts") + // shellQuote wraps the path in POSIX single-quotes to handle DataPath + // values that contain spaces without breaking the SSH argument list. + sshCommand := "ssh -o StrictHostKeyChecking=accept-new -o UserKnownHostsFile=" + shellQuote(knownHostsFile) + if sockPath := os.Getenv("SSH_AUTH_SOCK"); sockPath != "" { + // SSH_AUTH_SOCK is a Unix socket path from the process environment. + // It is not influenced by user input (mirror URLs are validated + // separately), so forwarding it here is safe. A newline in this + // value would malform the env block; os.Getenv strips NUL bytes + // but not newlines — reject the value if it contains one. + if strings.ContainsAny(sockPath, "\n\r\x00") { + b.logger.Warn("push-mirror: SSH_AUTH_SOCK contains unsafe characters, skipping agent forwarding") + } else { + cmd.Env = append(cmd.Env, "SSH_AUTH_SOCK="+sockPath) + } + } + cmd.Env = append(cmd.Env, "GIT_SSH_COMMAND="+sshCommand) + } + if out, err := cmd.CombinedOutput(); err != nil { + b.logger.Warn("push-mirror: push failed", "repo", repo.Name(), "mirror", m.Name, "err", err, "output", string(out)) + } else { + b.logger.Info("push-mirror: pushed", "repo", repo.Name(), "mirror", m.Name) + } + }(m) + } + // wg.Wait blocks until all concurrency-bounded pushes complete. + // PushMirrors itself is called from a goroutine in syncRepoMeta, + // so blocking here does not delay the push response to the git client. + wg.Wait() +} + +// extractSCPHost parses the host from an SCP-style git remote URL +// (e.g. git@host:repo or host:repo). Returns an error if no host can be +// determined. Strips IPv6 brackets and zone identifiers before returning. +func extractSCPHost(raw string) (string, error) { + if at := strings.LastIndex(raw, "@"); at != -1 { + raw = raw[at+1:] + } + colon := strings.LastIndex(raw, ":") + if colon == -1 { + return "", fmt.Errorf("cannot extract host from SCP-style remote (no colon): %q", raw) + } + host := raw[:colon] + host = strings.TrimPrefix(strings.TrimSuffix(host, "]"), "[") + // Strip IPv6 zone identifier (e.g. "::1%eth0" -> "::1") to prevent + // scoped addresses from bypassing the loopback check in ValidateHost. + if z := strings.IndexByte(host, '%'); z != -1 { + host = host[:z] + } + return host, nil +} diff --git a/pkg/backend/repo.go b/pkg/backend/repo.go index f9b70bbc3..730db13d0 100644 --- a/pkg/backend/repo.go +++ b/pkg/backend/repo.go @@ -19,31 +19,63 @@ import ( "github.com/charmbracelet/soft-serve/pkg/hooks" "github.com/charmbracelet/soft-serve/pkg/lfs" "github.com/charmbracelet/soft-serve/pkg/proto" + "github.com/charmbracelet/soft-serve/pkg/ssrf" "github.com/charmbracelet/soft-serve/pkg/storage" "github.com/charmbracelet/soft-serve/pkg/task" "github.com/charmbracelet/soft-serve/pkg/utils" "github.com/charmbracelet/soft-serve/pkg/webhook" ) -func validateImportRemote(remote string) error { +// shellQuote returns s wrapped in POSIX single-quotes with any embedded +// single-quote characters properly escaped as '\''. This is safe to embed +// inside GIT_SSH_COMMAND values that are evaluated by the shell git invokes. +func shellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" +} + +func validateImportRemote(ctx context.Context, remote string) error { endpoint, err := lfs.NewEndpoint(remote) if err != nil || endpoint.Host == "" { return proto.ErrInvalidRemote } + if endpoint.Scheme == "http" || endpoint.Scheme == "https" { + if err := ssrf.ValidateURL(ctx, remote); err != nil { + return err + } + } + + switch endpoint.Scheme { + case "ssh", "git+ssh", "ssh+git": + if err := ssrf.ValidateHost(ctx, endpoint.Host); err != nil { + return fmt.Errorf("import remote: %w", err) + } + } + return nil } // CreateRepository creates a new repository. // // It implements backend.Backend. -func (d *Backend) CreateRepository(ctx context.Context, name string, user proto.User, opts proto.RepositoryOptions) (proto.Repository, error) { +func (d *Backend) CreateRepository(ctx context.Context, name string, user proto.User, opts proto.RepositoryOptions) (_ proto.Repository, err error) { name = utils.SanitizeRepo(name) if err := utils.ValidateRepo(name); err != nil { return nil, err } - rp := filepath.Join(d.repoPath(name)) + rp := d.repoPath(name) + + // Clean up the repo directory if the transaction fails — git.Init + // creates OS files that the DB rollback cannot undo, which would leave + // an orphaned directory blocking future create attempts. + defer func() { + if err != nil { + if rmErr := os.RemoveAll(rp); rmErr != nil { + d.logger.Error("failed to clean up repo dir after failed create", "path", rp, "err", rmErr) + } + } + }() var userID int64 if user != nil { @@ -65,13 +97,34 @@ func (d *Backend) CreateRepository(ctx context.Context, name string, user proto. return err } - _, err := git.Init(rp, true) + // Skip git.Init when the directory is already a valid git repository + // (e.g. populated by git.Clone inside ImportRepository). Calling + // git.Init on an existing bare clone overwrites the config file, + // stripping mirror-specific settings (mirror=true, fetch refspecs). + rr, err := git.Open(rp) if err != nil { - d.logger.Debug("failed to create repository", "err", err) - return err + // Directory not yet a git repo — initialize it. + rr, err = git.Init(rp, true) + if err != nil { + d.logger.Debug("failed to create repository", "err", err) + return err + } + } + + if user != nil && user.Username() != "" { + rcfg, err := rr.Config() + if err != nil { + d.logger.Error("failed to get repository config", "repo", name, "err", err) + return err + } + rcfg.Section("gitweb").SetOption("owner", user.Username()) + if err := rr.SetConfig(rcfg); err != nil { + d.logger.Error("failed to set gitweb.owner", "repo", name, "err", err) + return err + } } - if err := os.WriteFile(filepath.Join(rp, "description"), []byte(opts.Description), fs.ModePerm); err != nil { + if err := os.WriteFile(filepath.Join(rp, "description"), []byte(opts.Description), 0o644); err != nil { d.logger.Error("failed to write description", "repo", name, "err", err) return err } @@ -88,7 +141,8 @@ func (d *Backend) CreateRepository(ctx context.Context, name string, user proto. d.logger.Debug("failed to create repository in database", "err", err) err = db.WrapError(err) if errors.Is(err, db.ErrDuplicateKey) { - return nil, proto.ErrRepoExist + err = proto.ErrRepoExist + return nil, err } return nil, err @@ -98,34 +152,48 @@ func (d *Backend) CreateRepository(ctx context.Context, name string, user proto. } // ImportRepository imports a repository from remote. -// XXX: This a expensive operation and should be run in a goroutine. -func (d *Backend) ImportRepository(_ context.Context, name string, user proto.User, remote string, opts proto.RepositoryOptions) (proto.Repository, error) { +// XXX: This is an expensive operation and should be run in a goroutine. +func (d *Backend) ImportRepository(ctx context.Context, name string, user proto.User, remote string, opts proto.RepositoryOptions) (proto.Repository, error) { name = utils.SanitizeRepo(name) if err := utils.ValidateRepo(name); err != nil { return nil, err } remote = utils.Sanitize(remote) - if err := validateImportRemote(remote); err != nil { + if err := validateImportRemote(ctx, remote); err != nil { return nil, err } - rp := filepath.Join(d.repoPath(name)) + rp := d.repoPath(name) + + if d.manager == nil { + return nil, fmt.Errorf("import repository: no manager configured") + } tid := "import:" + name if d.manager.Exists(tid) { return nil, task.ErrAlreadyStarted } - if _, err := os.Stat(rp); err == nil || os.IsExist(err) { + // Note: there is a TOCTOU between the Exists check, filesystem stat, and + // manager.Add. The task manager's LoadOrStore provides atomic dedup for + // concurrent callers, so this is safe for correctness; the stat check is + // advisory only. + // os.Stat returns nil (path exists) or fs.ErrNotExist (absent). It never + // returns fs.ErrExist, so the only relevant check is err == nil. + if _, err := os.Stat(rp); err == nil { return nil, proto.ErrRepoExist } done := make(chan error, 1) repoc := make(chan proto.Repository, 1) d.logger.Info("importing repository", "name", name, "remote", remote, "path", rp) - d.manager.Add(tid, func(ctx context.Context) (err error) { - ctx = proto.WithUserContext(ctx, user) + // Use context.WithoutCancel so the import task outlives the HTTP request + // that initiated it, but still inherits values (e.g. logger, config) from + // the caller's context. + importCtx := context.WithoutCancel(ctx) + d.manager.Add(tid, func(_ context.Context) (err error) { + ctx := proto.WithUserContext(importCtx, user) copts := git.CloneOptions{ Bare: true, @@ -135,9 +203,12 @@ func (d *Backend) ImportRepository(_ context.Context, name string, user proto.Us Timeout: -1, Context: ctx, Envs: []string{ - fmt.Sprintf(`GIT_SSH_COMMAND=ssh -o UserKnownHostsFile="%s" -o StrictHostKeyChecking=no -i "%s"`, - filepath.Join(d.cfg.DataPath, "ssh", "known_hosts"), - d.cfg.SSH.ClientKeyPath, + // Use shellQuote (POSIX single-quote wrapping) for both paths + // to prevent shell metacharacter expansion. GIT_SSH_COMMAND is + // interpreted by the shell that git invokes for the ssh call. + fmt.Sprintf("GIT_SSH_COMMAND=ssh -o UserKnownHostsFile=%s -o StrictHostKeyChecking=accept-new -i %s", + shellQuote(filepath.Join(d.cfg.DataPath, "ssh", "known_hosts")), + shellQuote(d.cfg.SSH.ClientKeyPath), ), }, }, @@ -150,34 +221,34 @@ func (d *Backend) ImportRepository(_ context.Context, name string, user proto.Us err = errors.Join(err, rerr) } + repoc <- nil return err } r, err := d.CreateRepository(ctx, name, user, opts) if err != nil { d.logger.Error("failed to create repository", "err", err, "name", name) + repoc <- nil return err } - defer func() { - if err != nil { - if rerr := d.DeleteRepository(ctx, name); rerr != nil { - d.logger.Error("failed to delete repository", "err", rerr, "name", name) - } - } - }() - rr, err := r.Open() if err != nil { d.logger.Error("failed to open repository", "err", err, "path", rp) + if rerr := d.DeleteRepository(ctx, name); rerr != nil { + d.logger.Error("failed to delete repository", "err", rerr, "name", name) + } + repoc <- nil return err } - repoc <- r - rcfg, err := rr.Config() if err != nil { d.logger.Error("failed to get repository config", "err", err, "path", rp) + if rerr := d.DeleteRepository(ctx, name); rerr != nil { + d.logger.Error("failed to delete repository", "err", rerr, "name", name) + } + repoc <- nil return err } @@ -190,26 +261,40 @@ func (d *Backend) ImportRepository(_ context.Context, name string, user proto.Us if err := rr.SetConfig(rcfg); err != nil { d.logger.Error("failed to set repository config", "err", err, "path", rp) + if rerr := d.DeleteRepository(ctx, name); rerr != nil { + d.logger.Error("failed to delete repository", "err", rerr, "name", name) + } + repoc <- nil return err } ep, err := lfs.NewEndpoint(endpoint) if err != nil { d.logger.Error("failed to create lfs endpoint", "err", err, "path", rp) + if rerr := d.DeleteRepository(ctx, name); rerr != nil { + d.logger.Error("failed to delete repository", "err", rerr, "name", name) + } + repoc <- nil return err } client := lfs.NewClient(ep) if client == nil { d.logger.Warn("failed to create lfs client: unsupported endpoint", "endpoint", endpoint) + repoc <- r return nil } if err := StoreRepoMissingLFSObjects(ctx, r, d.db, d.store, client); err != nil { d.logger.Error("failed to store missing lfs objects", "err", err, "path", rp) + if rerr := d.DeleteRepository(ctx, name); rerr != nil { + d.logger.Error("failed to delete repository", "err", rerr, "name", name) + } + repoc <- nil return err } + repoc <- r return nil }) @@ -218,7 +303,29 @@ func (d *Backend) ImportRepository(_ context.Context, name string, user proto.Us d.manager.Run(tid, done) }() - return <-repoc, <-done + // Use select: repoc and done are both buffered (cap 1). Normal path sends + // repoc first then done; cancellation path sends done first. + select { + case r := <-repoc: + return r, <-done + case err := <-done: + if err == nil { + // Import may have completed just before the context fired; + // return the repository if already produced. + select { + case r := <-repoc: + return r, nil + default: + // This branch should be unreachable: the import goroutine always + // sends to repoc before signalling done with a nil error. If it + // is reached, something is very wrong — log loudly so it is not + // silently swallowed. + d.logger.Error("import: done fired with nil error but repoc was empty — this is a bug") + return nil, fmt.Errorf("import: repository unavailable after completion") + } + } + return nil, err + } } // DeleteRepository deletes a repository. @@ -226,7 +333,7 @@ func (d *Backend) ImportRepository(_ context.Context, name string, user proto.Us // It implements backend.Backend. func (d *Backend) DeleteRepository(ctx context.Context, name string) error { name = utils.SanitizeRepo(name) - rp := filepath.Join(d.repoPath(name)) + rp := d.repoPath(name) user := proto.UserFromContext(ctx) r, err := d.Repository(ctx, name) @@ -241,21 +348,24 @@ func (d *Backend) DeleteRepository(ctx context.Context, name string) error { return err } + // removeDir is set to true inside the transaction only after all DB changes + // have succeeded. The actual filesystem removal happens after the transaction + // commits so that a DB commit failure cannot leave us with a missing directory + // and a stale DB row. + var removeDir bool if err := d.db.TransactionContext(ctx, func(tx *db.Tx) error { - // Delete repo from cache - defer d.cache.Delete(name) - repom, dberr := d.store.GetRepoByName(ctx, tx, name) _, ferr := os.Stat(rp) if dberr != nil && ferr != nil { return proto.ErrRepoNotFound } - // If the repo is not in the database but the directory exists, remove it - if dberr != nil && ferr == nil { - return os.RemoveAll(rp) - } else if dberr != nil { - return db.WrapError(dberr) + // If the repo is not in the database but the directory exists, mark it + // for removal after the transaction. The previous guard already returned + // when both are missing, so here dberr!=nil implies ferr==nil. + if dberr != nil { + removeDir = true + return nil } repoID := strconv.FormatInt(repom.ID, 10) @@ -265,6 +375,7 @@ func (d *Backend) DeleteRepository(ctx context.Context, name string) error { return db.WrapError(err) } + var lfsErrs []error for _, obj := range objs { p := lfs.Pointer{ Oid: obj.Oid, @@ -274,14 +385,22 @@ func (d *Backend) DeleteRepository(ctx context.Context, name string) error { d.logger.Debug("deleting lfs object", "repo", name, "oid", obj.Oid) if err := strg.Delete(path.Join("objects", p.RelativePath())); err != nil { d.logger.Error("failed to delete lfs object", "repo", name, "err", err, "oid", obj.Oid) + lfsErrs = append(lfsErrs, err) } } + if len(lfsErrs) > 0 { + return errors.Join(lfsErrs...) + } if err := d.store.DeleteRepoByName(ctx, tx, name); err != nil { return db.WrapError(err) } - return os.RemoveAll(rp) + // Mark for post-commit removal. The directory is only removed after the + // transaction commits so that a DB commit failure cannot leave us with + // the directory missing but the DB row still present. + removeDir = true + return nil }); err != nil { if errors.Is(err, db.ErrRecordNotFound) { return proto.ErrRepoNotFound @@ -290,11 +409,27 @@ func (d *Backend) DeleteRepository(ctx context.Context, name string) error { return db.WrapError(err) } + // The transaction has committed — it is now safe to remove the directory. + // os.RemoveAll is idempotent: if the directory is already absent (e.g. from + // a prior partial delete), this is a no-op. + if removeDir { + if err := os.RemoveAll(rp); err != nil { + // Non-fatal: DB row is already gone. Log and continue — the orphan + // directory will be cleaned up on the next delete attempt. + d.logger.Error("failed to remove repository directory after delete", "path", rp, "err", err) + } + } + + d.cache.Delete(name) + return webhook.SendEvent(ctx, wh) } // DeleteUserRepositories deletes all user repositories. func (d *Backend) DeleteUserRepositories(ctx context.Context, username string) error { + // Collect repo names inside a transaction, then delete outside it to avoid + // nested-transaction deadlock on SQLite. + var names []string if err := d.db.TransactionContext(ctx, func(tx *db.Tx) error { user, err := d.store.FindUserByUsername(ctx, tx, username) if err != nil { @@ -307,9 +442,7 @@ func (d *Backend) DeleteUserRepositories(ctx context.Context, username string) e } for _, repo := range repos { - if err := d.DeleteRepository(ctx, repo.Name); err != nil { - return err - } + names = append(names, repo.Name) } return nil @@ -317,6 +450,12 @@ func (d *Backend) DeleteUserRepositories(ctx context.Context, username string) e return db.WrapError(err) } + for _, name := range names { + if err := d.DeleteRepository(ctx, name); err != nil { + d.logger.Error("error deleting user repo", "repo", name, "err", err) + } + } + return nil } @@ -338,8 +477,8 @@ func (d *Backend) RenameRepository(ctx context.Context, oldName string, newName return nil } - op := filepath.Join(d.repoPath(oldName)) - np := filepath.Join(d.repoPath(newName)) + op := d.repoPath(oldName) + np := d.repoPath(newName) if _, err := os.Stat(op); err != nil { return proto.ErrRepoNotFound } @@ -384,33 +523,41 @@ func (d *Backend) RenameRepository(ctx context.Context, oldName string, newName // // It implements backend.Backend. func (d *Backend) Repositories(ctx context.Context) ([]proto.Repository, error) { - repos := make([]proto.Repository, 0) - - if err := d.db.TransactionContext(ctx, func(tx *db.Tx) error { - ms, err := d.store.GetAllRepos(ctx, tx) - if err != nil { - return err - } - - for _, m := range ms { - r := &repo{ - name: m.Name, - path: filepath.Join(d.repoPath(m.Name)), - repo: m, + // Use singleflight to coalesce concurrent calls and prevent cache stampede. + // The lambda uses d.ctx (backend root context) rather than the caller's ctx + // so that one caller cancelling does not abort the shared in-flight query + // for all other waiters. + // + // Note: the result is a snapshot at query time and is populated into d.cache + // (per-repo entries) without an expiry. High-churn environments (frequent + // create/delete) should be aware that Repositories() reflects the state at + // the time of the most recent coalesced query until the next call. + v, err, _ := d.reposSFG.Do("all", func() (interface{}, error) { + repos := make([]proto.Repository, 0) + if err := d.db.TransactionContext(d.ctx, func(tx *db.Tx) error { + ms, err := d.store.GetAllRepos(d.ctx, tx) + if err != nil { + return err } - - // Cache repositories - d.cache.Set(m.Name, r) - - repos = append(repos, r) + for _, m := range ms { + r := &repo{ + name: m.Name, + path: d.repoPath(m.Name), + repo: m, + } + d.cache.Set(m.Name, r) + repos = append(repos, r) + } + return nil + }); err != nil { + return nil, db.WrapError(err) } - - return nil - }); err != nil { - return nil, db.WrapError(err) + return repos, nil + }) + if err != nil { + return nil, err } - - return repos, nil + return v.([]proto.Repository), nil } // Repository returns a repository by name. @@ -424,7 +571,7 @@ func (d *Backend) Repository(ctx context.Context, name string) (proto.Repository return r, nil } - rp := filepath.Join(d.repoPath(name)) + rp := d.repoPath(name) if _, err := os.Stat(rp); err != nil { if !errors.Is(err, fs.ErrNotExist) { d.logger.Errorf("failed to stat repository path: %v", err) @@ -559,13 +706,13 @@ func (d *Backend) SetHidden(ctx context.Context, name string, hidden bool) error func (d *Backend) SetDescription(ctx context.Context, name string, desc string) error { name = utils.SanitizeRepo(name) desc = utils.Sanitize(desc) - rp := filepath.Join(d.repoPath(name)) + rp := d.repoPath(name) // Delete cache d.cache.Delete(name) return d.db.TransactionContext(ctx, func(tx *db.Tx) error { - if err := os.WriteFile(filepath.Join(rp, "description"), []byte(desc), fs.ModePerm); err != nil { + if err := os.WriteFile(filepath.Join(rp, "description"), []byte(desc), 0o644); err != nil { d.logger.Error("failed to write description", "repo", name, "err", err) return err } @@ -579,7 +726,14 @@ func (d *Backend) SetDescription(ctx context.Context, name string, desc string) // It implements backend.Backend. func (d *Backend) SetPrivate(ctx context.Context, name string, private bool) error { name = utils.SanitizeRepo(name) - rp := filepath.Join(d.repoPath(name)) + rp := d.repoPath(name) + + // Capture the old visibility before updating. + oldRepo, err := d.Repository(ctx, name) + if err != nil { + return err + } + oldPrivate := oldRepo.IsPrivate() // Delete cache d.cache.Delete(name) @@ -613,7 +767,7 @@ func (d *Backend) SetPrivate(ctx context.Context, name string, private bool) err return err } - if repo.IsPrivate() != !private { + if oldPrivate != private { wh, err := webhook.NewRepositoryEvent(ctx, user, repo, webhook.RepositoryEventActionVisibilityChange) if err != nil { return err diff --git a/pkg/backend/settings.go b/pkg/backend/settings.go index 3b7ddbf18..400702e3d 100644 --- a/pkg/backend/settings.go +++ b/pkg/backend/settings.go @@ -2,40 +2,92 @@ package backend import ( "context" + "sync" + "time" "github.com/charmbracelet/soft-serve/pkg/access" "github.com/charmbracelet/soft-serve/pkg/db" ) +// cachedBool is a simple time-based cache for a boolean value. +type cachedBool struct { + mu sync.Mutex + val bool + expiresAt time.Time +} + +func (c *cachedBool) get(ttl time.Duration, fetch func() (bool, error)) (bool, error) { + c.mu.Lock() + defer c.mu.Unlock() + if time.Now().Before(c.expiresAt) { + return c.val, nil + } + v, err := fetch() + if err != nil { + return false, err + } + c.val = v + c.expiresAt = time.Now().Add(ttl) + return v, nil +} + +const settingsCacheTTL = 30 * time.Second + +var ( + allowKeylessCache cachedBool + anonAccessCache struct { + mu sync.Mutex + val access.AccessLevel + expiresAt time.Time + } +) + // AllowKeyless returns whether or not keyless access is allowed. // // It implements backend.Backend. func (b *Backend) AllowKeyless(ctx context.Context) bool { - var allow bool - if err := b.db.TransactionContext(ctx, func(tx *db.Tx) error { - var err error - allow, err = b.store.GetAllowKeylessAccess(ctx, tx) - return err - }); err != nil { + val, err := allowKeylessCache.get(settingsCacheTTL, func() (bool, error) { + var allow bool + if err := b.db.TransactionContext(ctx, func(tx *db.Tx) error { + var err error + allow, err = b.store.GetAllowKeylessAccess(ctx, tx) + return err + }); err != nil { + return false, err + } + return allow, nil + }) + if err != nil { return false } - - return allow + return val } // SetAllowKeyless sets whether or not keyless access is allowed. // // It implements backend.Backend. func (b *Backend) SetAllowKeyless(ctx context.Context, allow bool) error { - return b.db.TransactionContext(ctx, func(tx *db.Tx) error { + if err := b.db.TransactionContext(ctx, func(tx *db.Tx) error { return b.store.SetAllowKeylessAccess(ctx, tx, allow) - }) + }); err != nil { + return err + } + // Invalidate cache on write. + allowKeylessCache.mu.Lock() + allowKeylessCache.expiresAt = time.Time{} + allowKeylessCache.mu.Unlock() + return nil } // AnonAccess returns the level of anonymous access. // // It implements backend.Backend. func (b *Backend) AnonAccess(ctx context.Context) access.AccessLevel { + anonAccessCache.mu.Lock() + defer anonAccessCache.mu.Unlock() + if time.Now().Before(anonAccessCache.expiresAt) { + return anonAccessCache.val + } var level access.AccessLevel if err := b.db.TransactionContext(ctx, func(tx *db.Tx) error { var err error @@ -44,7 +96,8 @@ func (b *Backend) AnonAccess(ctx context.Context) access.AccessLevel { }); err != nil { return access.NoAccess } - + anonAccessCache.val = level + anonAccessCache.expiresAt = time.Now().Add(settingsCacheTTL) return level } @@ -52,7 +105,14 @@ func (b *Backend) AnonAccess(ctx context.Context) access.AccessLevel { // // It implements backend.Backend. func (b *Backend) SetAnonAccess(ctx context.Context, level access.AccessLevel) error { - return b.db.TransactionContext(ctx, func(tx *db.Tx) error { + if err := b.db.TransactionContext(ctx, func(tx *db.Tx) error { return b.store.SetAnonAccess(ctx, tx, level) - }) + }); err != nil { + return err + } + // Invalidate cache on write. + anonAccessCache.mu.Lock() + anonAccessCache.expiresAt = time.Time{} + anonAccessCache.mu.Unlock() + return nil } diff --git a/pkg/backend/user.go b/pkg/backend/user.go index 736842f9f..53a396163 100644 --- a/pkg/backend/user.go +++ b/pkg/backend/user.go @@ -23,24 +23,6 @@ func (d *Backend) AccessLevel(ctx context.Context, repo string, username string) return d.AccessLevelForUser(ctx, repo, user) } -// AccessLevelByPublicKey returns the access level of a user's public key for a repository. -// -// It implements backend.Backend. -func (d *Backend) AccessLevelByPublicKey(ctx context.Context, repo string, pk ssh.PublicKey) access.AccessLevel { - for _, k := range d.cfg.AdminKeys() { - if sshutils.KeysEqual(pk, k) { - return access.AdminAccess - } - } - - user, _ := d.UserByPublicKey(ctx, pk) - if user != nil { - return d.AccessLevel(ctx, repo, user.Username()) - } - - return d.AccessLevel(ctx, repo, "") -} - // AccessLevelForUser returns the access level of a user for a repository. // TODO: user repository ownership func (d *Backend) AccessLevelForUser(ctx context.Context, repo string, user proto.User) access.AccessLevel { @@ -226,7 +208,7 @@ func (d *Backend) UserByAccessToken(ctx context.Context, token string) (proto.Us if errors.Is(err, db.ErrRecordNotFound) { return nil, proto.ErrUserNotFound } - d.logger.Error("failed to find user by access token", "err", err, "token", token) + d.logger.Error("failed to find user by access token", "err", err) return nil, err } @@ -298,18 +280,51 @@ func (d *Backend) CreateUser(ctx context.Context, username string, opts proto.Us // // It implements backend.Backend. func (d *Backend) DeleteUser(ctx context.Context, username string) error { + username = utils.Sanitize(username) username = strings.ToLower(username) if err := utils.ValidateUsername(username); err != nil { return err } - return d.db.TransactionContext(ctx, func(tx *db.Tx) error { - if err := d.store.DeleteUserByUsername(ctx, tx, username); err != nil { + // Collect the user's repo names and delete the user record in one transaction. + var repoNames []string + if err := d.db.TransactionContext(ctx, func(tx *db.Tx) error { + u, err := d.store.FindUserByUsername(ctx, tx, username) + if err != nil { return db.WrapError(err) } - return d.DeleteUserRepositories(ctx, username) - }) + repos, err := d.store.GetUserRepos(ctx, tx, u.ID) + if err != nil { + return db.WrapError(err) + } + + for _, r := range repos { + repoNames = append(repoNames, r.Name) + } + + return db.WrapError(d.store.DeleteUserByUsername(ctx, tx, username)) + }); err != nil { + return err + } + + // NOTE: There is a window between the user-record deletion and repository + // deletions below where orphaned repos have no owner. A future improvement + // would be to soft-delete repos atomically inside the user-deletion transaction + // so they become invisible immediately before filesystem cleanup. + // Repos are deleted outside the transaction to avoid nested-transaction issues. + var errs []error + for _, name := range repoNames { + if err := d.DeleteRepository(ctx, name); err != nil { + d.logger.Error("error deleting user repo", "repo", name, "err", err) + errs = append(errs, err) + } + } + if len(errs) > 0 { + return errors.Join(errs...) + } + + return nil } // RemovePublicKey removes a public key from a user. @@ -351,6 +366,11 @@ func (d *Backend) SetUsername(ctx context.Context, username string, newUsername return err } + newUsername = strings.ToLower(newUsername) + if err := utils.ValidateUsername(newUsername); err != nil { + return err + } + return db.WrapError( d.db.TransactionContext(ctx, func(tx *db.Tx) error { return d.store.SetUsernameByUsername(ctx, tx, username, newUsername) @@ -362,6 +382,7 @@ func (d *Backend) SetUsername(ctx context.Context, username string, newUsername // // It implements backend.Backend. func (d *Backend) SetAdmin(ctx context.Context, username string, admin bool) error { + username = utils.Sanitize(username) username = strings.ToLower(username) if err := utils.ValidateUsername(username); err != nil { return err diff --git a/pkg/backend/utils.go b/pkg/backend/utils.go index be1b0b4a7..03939c9e6 100644 --- a/pkg/backend/utils.go +++ b/pkg/backend/utils.go @@ -1,6 +1,8 @@ package backend import ( + "errors" + "github.com/charmbracelet/soft-serve/git" "github.com/charmbracelet/soft-serve/pkg/proto" ) @@ -15,9 +17,38 @@ func LatestFile(r proto.Repository, ref *git.Reference, pattern string) (string, return git.LatestFile(repo, ref, pattern) } +// readmePatterns is the ordered list of glob patterns used to find a README. +// Root-level patterns are checked first; subdirectory paths are fallbacks, +// matching GitHub's README discovery behavior. +var readmePatterns = []string{ + "[rR][eE][aA][dD][mM][eE]", + "[rR][eE][aA][dD][mM][eE].*", + "docs/[rR][eE][aA][dD][mM][eE]", + "docs/[rR][eE][aA][dD][mM][eE].*", + ".github/[rR][eE][aA][dD][mM][eE]", + ".github/[rR][eE][aA][dD][mM][eE].*", + ".gitlab/[rR][eE][aA][dD][mM][eE]", + ".gitlab/[rR][eE][aA][dD][mM][eE].*", +} + // Readme returns the repository's README. +// It checks the repository root first, then falls back to docs/, .github/, +// and .gitlab/ subdirectories. +// When no README is found in any location, it returns ("", "", nil). +// Callers should check whether path is empty to detect a missing README. func Readme(r proto.Repository, ref *git.Reference) (readme string, path string, err error) { - pattern := "[rR][eE][aA][dD][mM][eE]*" - readme, path, err = LatestFile(r, ref, pattern) - return + for _, pattern := range readmePatterns { + readme, path, err = LatestFile(r, ref, pattern) + if err == nil { + return + } + if !errors.Is(err, git.ErrFileNotFound) && + !errors.Is(err, git.ErrRevisionNotExist) && + !errors.Is(err, git.ErrReferenceNotExist) { + return + } + } + // No README found in any location; return a clean sentinel rather than + // leaking the error from the last pattern tried. + return "", "", nil } diff --git a/pkg/backend/utils_test.go b/pkg/backend/utils_test.go new file mode 100644 index 000000000..bf05f99ff --- /dev/null +++ b/pkg/backend/utils_test.go @@ -0,0 +1,213 @@ +package backend + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/charmbracelet/soft-serve/git" +) + +// stubRepo implements proto.Repository backed by a real on-disk git repo. +type stubRepo struct { + path string +} + +func (s *stubRepo) ID() int64 { return 0 } +func (s *stubRepo) Name() string { return "stub" } +func (s *stubRepo) ProjectName() string { return "stub" } +func (s *stubRepo) Description() string { return "" } +func (s *stubRepo) IsPrivate() bool { return false } +func (s *stubRepo) IsMirror() bool { return false } +func (s *stubRepo) IsHidden() bool { return false } +func (s *stubRepo) UserID() int64 { return 0 } +func (s *stubRepo) CreatedAt() time.Time { return time.Time{} } +func (s *stubRepo) UpdatedAt() time.Time { return time.Time{} } +func (s *stubRepo) Open() (*git.Repository, error) { + return git.Open(s.path) +} + +// initTestRepo creates a non-bare git repo in a temp dir, commits the given +// files (map of relative path → content), and returns the path. +func initTestRepo(t *testing.T, files map[string]string) string { + t.Helper() + dir := t.TempDir() + + repo, err := git.Init(dir, false) + if err != nil { + t.Fatalf("git init: %v", err) + } + _ = repo + + for rel, content := range files { + full := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatalf("write file %s: %v", rel, err) + } + } + + // git add -A && git commit + cmd := func(args ...string) { + t.Helper() + c := git.NewCommand(args...) + if _, err := c.RunInDir(dir); err != nil { + t.Fatalf("git %v: %v", args, err) + } + } + cmd("add", "-A") + cmd("-c", "user.email=test@test.com", "-c", "user.name=Test", "commit", "-m", "init") + + return dir +} + +func TestReadme_Root(t *testing.T) { + dir := initTestRepo(t, map[string]string{ + "README.md": "# Root Readme", + }) + repo := &stubRepo{path: dir} + content, path, err := Readme(repo, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != "README.md" { + t.Errorf("expected path README.md, got %q", path) + } + if content != "# Root Readme" { + t.Errorf("unexpected content: %q", content) + } +} + +func TestReadme_DocsSubdir(t *testing.T) { + // No root README; only docs/README.md + dir := initTestRepo(t, map[string]string{ + "main.go": "package main", + "docs/README.md": "# Docs Readme", + }) + repo := &stubRepo{path: dir} + content, path, err := Readme(repo, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != "docs/README.md" { + t.Errorf("expected path docs/README.md, got %q", path) + } + if content != "# Docs Readme" { + t.Errorf("unexpected content: %q", content) + } +} + +func TestReadme_GithubSubdir(t *testing.T) { + // No root README; only .github/README.md + dir := initTestRepo(t, map[string]string{ + "main.go": "package main", + ".github/README.md": "# GitHub Readme", + }) + repo := &stubRepo{path: dir} + content, path, err := Readme(repo, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != ".github/README.md" { + t.Errorf("expected path .github/README.md, got %q", path) + } + if content != "# GitHub Readme" { + t.Errorf("unexpected content: %q", content) + } +} + +func TestReadme_GitlabSubdir(t *testing.T) { + // No root README; only .gitlab/README.md + dir := initTestRepo(t, map[string]string{ + "main.go": "package main", + ".gitlab/README.md": "# GitLab Readme", + }) + repo := &stubRepo{path: dir} + content, path, err := Readme(repo, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != ".gitlab/README.md" { + t.Errorf("expected path .gitlab/README.md, got %q", path) + } + if content != "# GitLab Readme" { + t.Errorf("unexpected content: %q", content) + } +} + +func TestReadme_RootTakesPrecedence(t *testing.T) { + // Both root and docs have a README; root should win + dir := initTestRepo(t, map[string]string{ + "README.md": "# Root Readme", + "docs/README.md": "# Docs Readme", + }) + repo := &stubRepo{path: dir} + _, path, err := Readme(repo, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != "README.md" { + t.Errorf("expected root README.md to take precedence, got %q", path) + } +} + +func TestReadme_NotFound(t *testing.T) { + dir := initTestRepo(t, map[string]string{ + "main.go": "package main", + }) + repo := &stubRepo{path: dir} + content, path, err := Readme(repo, nil) + if err != nil { + t.Fatalf("unexpected error when no readme exists: %v", err) + } + if path != "" || content != "" { + t.Errorf("expected empty path and content when no readme exists, got content=%q path=%q", content, path) + } +} + +// initBareTestRepo creates a bare git repo in a temp dir by initialising a +// non-bare repo, committing the given files, and then cloning it as bare. +func initBareTestRepo(t *testing.T, files map[string]string) string { + t.Helper() + src := initTestRepo(t, files) + bareDir := t.TempDir() + c := git.NewCommand("clone", "--bare", src, bareDir) + if _, err := c.Run(); err != nil { + t.Fatalf("git clone --bare: %v", err) + } + return bareDir +} + +func TestReadme_BareRepo_Root(t *testing.T) { + dir := initBareTestRepo(t, map[string]string{ + "README.md": "# Bare Readme", + }) + repo := &stubRepo{path: dir} + content, path, err := Readme(repo, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != "README.md" { + t.Errorf("expected path README.md, got %q", path) + } + if content != "# Bare Readme" { + t.Errorf("unexpected content: %q", content) + } +} + +func TestReadme_BareRepo_NotFound(t *testing.T) { + dir := initBareTestRepo(t, map[string]string{ + "main.go": "package main", + }) + repo := &stubRepo{path: dir} + content, path, err := Readme(repo, nil) + if err != nil { + t.Fatalf("unexpected error when no readme exists in bare repo: %v", err) + } + if path != "" || content != "" { + t.Errorf("expected empty result for bare repo with no readme, got content=%q path=%q", content, path) + } +} diff --git a/pkg/backend/webhooks.go b/pkg/backend/webhooks.go index 2fd90c2a5..9a86981d6 100644 --- a/pkg/backend/webhooks.go +++ b/pkg/backend/webhooks.go @@ -20,8 +20,13 @@ func (b *Backend) CreateWebhook(ctx context.Context, repo proto.Repository, url datastore := store.FromContext(ctx) url = utils.Sanitize(url) + if secret == "" { + logger := log.FromContext(ctx) + logger.Warn("webhook created without a secret — deliveries will not be signed and cannot be verified by the receiver", "url", url) + } + // Validate webhook URL to prevent SSRF attacks - if err := webhook.ValidateWebhookURL(url); err != nil { + if err := webhook.ValidateWebhookURL(ctx, url); err != nil { return err //nolint:wrapcheck } @@ -90,12 +95,20 @@ func (b *Backend) ListWebhooks(ctx context.Context, repo proto.Repository) ([]we return err } - for _, h := range webhooks { - events, err := datastore.GetWebhookEventsByWebhookID(ctx, tx, h.ID) - if err != nil { - return err - } - webhookEvents[h.ID] = events + if len(webhooks) == 0 { + return nil + } + + ids := make([]int64, len(webhooks)) + for i, h := range webhooks { + ids[i] = h.ID + } + allEvents, err := datastore.GetWebhookEventsByWebhookIDs(ctx, tx, ids) + if err != nil { + return err + } + for _, e := range allEvents { + webhookEvents[e.WebhookID] = append(webhookEvents[e.WebhookID], e) } return nil @@ -125,8 +138,10 @@ func (b *Backend) UpdateWebhook(ctx context.Context, repo proto.Repository, id i dbx := db.FromContext(ctx) datastore := store.FromContext(ctx) - // Validate webhook URL to prevent SSRF attacks - if err := webhook.ValidateWebhookURL(url); err != nil { + // Sanitize and validate webhook URL — mirrors CreateWebhook which also + // calls utils.Sanitize before ValidateWebhookURL. + url = utils.Sanitize(url) + if err := webhook.ValidateWebhookURL(ctx, url); err != nil { return err } @@ -140,42 +155,43 @@ func (b *Backend) UpdateWebhook(ctx context.Context, repo proto.Repository, id i return db.WrapError(err) } + // Build a set of current events for O(1) lookups. + currentSet := make(map[int]int64, len(currentEvents)) + for _, e := range currentEvents { + currentSet[e.Event] = e.ID + } + // Delete events that are no longer in the list. - toBeDeleted := make([]int64, 0) + updatedSet := make(map[webhook.Event]bool, len(updatedEvents)) + for _, e := range updatedEvents { + updatedSet[e] = true + } + var toBeDeleted []int64 for _, e := range currentEvents { - found := false - for _, ne := range updatedEvents { - if int(ne) == e.Event { - found = true - break - } - } - if !found { + if !updatedSet[webhook.Event(e.Event)] { toBeDeleted = append(toBeDeleted, e.ID) } } - if err := datastore.DeleteWebhookEventsByID(ctx, tx, toBeDeleted); err != nil { - return db.WrapError(err) + // Guard the delete call: sqlx.In returns an error for empty slices. + if len(toBeDeleted) > 0 { + if err := datastore.DeleteWebhookEventsByID(ctx, tx, toBeDeleted); err != nil { + return db.WrapError(err) + } } // Prune events that are already in the list. - newEvents := make([]int, 0) + var newEvents []int for _, e := range updatedEvents { - found := false - for _, ne := range currentEvents { - if int(e) == ne.Event { - found = true - break - } - } - if !found { + if _, exists := currentSet[int(e)]; !exists { newEvents = append(newEvents, int(e)) } } - if err := datastore.CreateWebhookEvents(ctx, tx, id, newEvents); err != nil { - return db.WrapError(err) + if len(newEvents) > 0 { + if err := datastore.CreateWebhookEvents(ctx, tx, id, newEvents); err != nil { + return db.WrapError(err) + } } return nil @@ -201,21 +217,28 @@ func (b *Backend) DeleteWebhook(ctx context.Context, repo proto.Repository, id i } // ListWebhookDeliveries lists webhook deliveries for a webhook. -func (b *Backend) ListWebhookDeliveries(ctx context.Context, id int64) ([]webhook.Delivery, error) { +// It verifies that the webhook belongs to the given repository before returning +// results to prevent cross-repo information leakage. +func (b *Backend) ListWebhookDeliveries(ctx context.Context, repo proto.Repository, id int64) ([]webhook.Delivery, error) { dbx := db.FromContext(ctx) datastore := store.FromContext(ctx) var deliveries []models.WebhookDelivery if err := dbx.TransactionContext(ctx, func(tx *db.Tx) error { - var err error - deliveries, err = datastore.ListWebhookDeliveriesByWebhookID(ctx, tx, id) - if err != nil { + // Verify webhook belongs to this repository before listing deliveries. + if _, err := datastore.GetWebhookByID(ctx, tx, repo.ID(), id); err != nil { return db.WrapError(err) } + var lerr error + deliveries, lerr = datastore.ListWebhookDeliveriesByWebhookID(ctx, tx, id) + if lerr != nil { + return db.WrapError(lerr) + } + return nil }); err != nil { - return nil, db.WrapError(err) + return nil, err } ds := make([]webhook.Delivery, len(deliveries)) @@ -240,7 +263,7 @@ func (b *Backend) RedeliverWebhookDelivery(ctx context.Context, repo proto.Repos var err error wh, err = datastore.GetWebhookByID(ctx, tx, repo.ID(), id) if err != nil { - log.Errorf("error getting webhook: %v", err) + b.logger.Errorf("error getting webhook: %v", err) return db.WrapError(err) } @@ -254,11 +277,11 @@ func (b *Backend) RedeliverWebhookDelivery(ctx context.Context, repo proto.Repos return db.WrapError(err) } - log.Infof("redelivering webhook delivery %s for webhook %d\n\n%s\n\n", delID, id, delivery.RequestBody) + b.logger.Infof("redelivering webhook delivery %s for webhook %d", delID, id) var payload json.RawMessage if err := json.Unmarshal([]byte(delivery.RequestBody), &payload); err != nil { - log.Errorf("error unmarshaling webhook payload: %v", err) + b.logger.Errorf("error unmarshaling webhook payload: %v", err) return err } @@ -266,12 +289,19 @@ func (b *Backend) RedeliverWebhookDelivery(ctx context.Context, repo proto.Repos } // WebhookDelivery returns a webhook delivery. -func (b *Backend) WebhookDelivery(ctx context.Context, webhookID int64, id uuid.UUID) (webhook.Delivery, error) { +// It verifies that the webhook belongs to the given repository before +// returning the delivery to prevent cross-repo information leakage. +func (b *Backend) WebhookDelivery(ctx context.Context, repo proto.Repository, webhookID int64, id uuid.UUID) (webhook.Delivery, error) { dbx := db.FromContext(ctx) datastore := store.FromContext(ctx) var delivery webhook.Delivery if err := dbx.TransactionContext(ctx, func(tx *db.Tx) error { + // Verify webhook belongs to this repository before fetching delivery. + if _, err := datastore.GetWebhookByID(ctx, tx, repo.ID(), webhookID); err != nil { + return db.WrapError(err) + } + d, err := datastore.GetWebhookDeliveryByID(ctx, tx, webhookID, id) if err != nil { return db.WrapError(err) diff --git a/pkg/config/config.go b/pkg/config/config.go index 97a5dc43d..ae4d033c3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -9,6 +9,7 @@ import ( "time" "github.com/caarlos0/env/v11" + "github.com/charmbracelet/soft-serve/pkg/access" "github.com/charmbracelet/soft-serve/pkg/sshutils" "golang.org/x/crypto/ssh" "gopkg.in/yaml.v3" @@ -38,6 +39,24 @@ type SSHConfig struct { // IdleTimeout is the number of seconds a connection can be idle before it is closed. IdleTimeout int `env:"IDLE_TIMEOUT" yaml:"idle_timeout"` + + // AllowMouseEvents controls whether the TUI enables mouse-event capture. + // When true (the default), mouse clicks and scrolling work in the TUI but + // the terminal's native text-selection is disabled. Set to false to + // restore terminal text selection at the cost of mouse-driven navigation. + AllowMouseEvents bool `env:"ALLOW_MOUSE_EVENTS" yaml:"allow_mouse_events"` + + // KeyExchanges is the list of key exchange algorithms to use. + // When empty the server uses the go/crypto defaults. + KeyExchanges []string `env:"KEY_EXCHANGES" envSeparator:"," yaml:"key_exchanges"` + + // Ciphers is the list of ciphers to use. + // When empty the server uses the go/crypto defaults. + Ciphers []string `env:"CIPHERS" envSeparator:"," yaml:"ciphers"` + + // MACs is the list of MAC algorithms to use. + // When empty the server uses the go/crypto defaults. + MACs []string `env:"MACS" envSeparator:"," yaml:"macs"` } // GitConfig is the Git daemon configuration for the server. @@ -89,6 +108,23 @@ type HTTPConfig struct { // CORS is the cross-origin configuration for the HTTP server. CORS CORSConfig `envPrefix:"CORS_" yaml:"cors"` + + // StripGitSuffix allows cloning repos without the .git suffix in the URL. + // When true, both / and /.git are accepted. + // Default is false for backward compatibility. + StripGitSuffix bool `env:"STRIP_GIT_SUFFIX" yaml:"strip_git_suffix"` + + // TrustProxyHeaders controls whether the X-Forwarded-For header is trusted + // for client IP resolution. Only enable this when the server sits behind a + // trusted reverse proxy. Default is false. + TrustProxyHeaders bool `env:"TRUST_PROXY_HEADERS" yaml:"trust_proxy_headers"` + + // RateLimit is the maximum number of HTTP requests per second per IP. + // Set to 0 to disable HTTP rate limiting. + RateLimit float64 `env:"RATE_LIMIT" yaml:"rate_limit"` + + // RateBurst is the maximum burst size for the HTTP rate limiter. + RateBurst int `env:"RATE_BURST" yaml:"rate_burst"` } // StatsConfig is the configuration for the stats server. @@ -134,9 +170,18 @@ type LFSConfig struct { SSHEnabled bool `env:"SSH_ENABLED" yaml:"ssh_enabled"` } +// MirrorPullJobConfig is the configuration for the mirror pull cron job. +type MirrorPullJobConfig struct { + // Enabled toggles the mirror pull job on/off. + Enabled bool `env:"ENABLED" yaml:"enabled"` + + // Schedule is the cron schedule for the mirror pull job. + Schedule string `env:"SCHEDULE" yaml:"schedule"` +} + // JobsConfig is the configuration for cron jobs. type JobsConfig struct { - MirrorPull string `env:"MIRROR_PULL" yaml:"mirror_pull"` + MirrorPull MirrorPullJobConfig `envPrefix:"MIRROR_PULL_" yaml:"mirror_pull"` } // Config is the configuration for Soft Serve. @@ -171,6 +216,20 @@ type Config struct { // InitialAdminKeys is a list of public keys that will be added to the list of admins. InitialAdminKeys []string `env:"INITIAL_ADMIN_KEYS" envSeparator:"\n" yaml:"initial_admin_keys"` + // AnonAccess is the anonymous access level applied on startup. + // Valid values: "no-access", "read-only", "read-write", "admin-access". + // Leave empty to keep the value stored in the database. + AnonAccess string `env:"ANON_ACCESS" yaml:"anon_access"` + + // AllowKeyless controls whether keyless (keyboard-interactive) access is + // allowed. When set, this overrides the value stored in the database on + // every startup. Leave unset to keep the database value. + AllowKeyless *bool `env:"ALLOW_KEYLESS" yaml:"allow_keyless"` + + // AllowPublicGoGet serves go-get meta tags for private/hidden repos when true. + // The actual git content remains inaccessible without authentication. + AllowPublicGoGet bool `env:"ALLOW_PUBLIC_GO_GET" yaml:"allow_public_go_get"` + // DataPath is the path to the directory where Soft Serve will store its data. DataPath string `env:"DATA_PATH" yaml:"-"` } @@ -190,6 +249,7 @@ func (c *Config) Environ() []string { fmt.Sprintf("SOFT_SERVE_DATA_PATH=%s", c.DataPath), fmt.Sprintf("SOFT_SERVE_NAME=%s", c.Name), fmt.Sprintf("SOFT_SERVE_INITIAL_ADMIN_KEYS=%s", strings.Join(c.InitialAdminKeys, "\n")), + fmt.Sprintf("SOFT_SERVE_ANON_ACCESS=%s", c.AnonAccess), fmt.Sprintf("SOFT_SERVE_SSH_ENABLED=%t", c.SSH.Enabled), fmt.Sprintf("SOFT_SERVE_SSH_LISTEN_ADDR=%s", c.SSH.ListenAddr), fmt.Sprintf("SOFT_SERVE_SSH_PUBLIC_URL=%s", c.SSH.PublicURL), @@ -197,6 +257,9 @@ func (c *Config) Environ() []string { fmt.Sprintf("SOFT_SERVE_SSH_CLIENT_KEY_PATH=%s", c.SSH.ClientKeyPath), fmt.Sprintf("SOFT_SERVE_SSH_MAX_TIMEOUT=%d", c.SSH.MaxTimeout), fmt.Sprintf("SOFT_SERVE_SSH_IDLE_TIMEOUT=%d", c.SSH.IdleTimeout), + fmt.Sprintf("SOFT_SERVE_SSH_KEY_EXCHANGES=%s", strings.Join(c.SSH.KeyExchanges, ",")), + fmt.Sprintf("SOFT_SERVE_SSH_CIPHERS=%s", strings.Join(c.SSH.Ciphers, ",")), + fmt.Sprintf("SOFT_SERVE_SSH_MACS=%s", strings.Join(c.SSH.MACs, ",")), fmt.Sprintf("SOFT_SERVE_GIT_ENABLED=%t", c.Git.Enabled), fmt.Sprintf("SOFT_SERVE_GIT_LISTEN_ADDR=%s", c.Git.ListenAddr), fmt.Sprintf("SOFT_SERVE_GIT_PUBLIC_URL=%s", c.Git.PublicURL), @@ -208,6 +271,7 @@ func (c *Config) Environ() []string { fmt.Sprintf("SOFT_SERVE_HTTP_TLS_KEY_PATH=%s", c.HTTP.TLSKeyPath), fmt.Sprintf("SOFT_SERVE_HTTP_TLS_CERT_PATH=%s", c.HTTP.TLSCertPath), fmt.Sprintf("SOFT_SERVE_HTTP_PUBLIC_URL=%s", c.HTTP.PublicURL), + fmt.Sprintf("SOFT_SERVE_HTTP_STRIP_GIT_SUFFIX=%t", c.HTTP.StripGitSuffix), fmt.Sprintf("SOFT_SERVE_HTTP_CORS_ALLOWED_HEADERS=%s", strings.Join(c.HTTP.CORS.AllowedHeaders, ",")), fmt.Sprintf("SOFT_SERVE_HTTP_CORS_ALLOWED_ORIGINS=%s", strings.Join(c.HTTP.CORS.AllowedOrigins, ",")), fmt.Sprintf("SOFT_SERVE_HTTP_CORS_ALLOWED_METHODS=%s", strings.Join(c.HTTP.CORS.AllowedMethods, ",")), @@ -219,7 +283,9 @@ func (c *Config) Environ() []string { fmt.Sprintf("SOFT_SERVE_DB_DATA_SOURCE=%s", c.DB.DataSource), fmt.Sprintf("SOFT_SERVE_LFS_ENABLED=%t", c.LFS.Enabled), fmt.Sprintf("SOFT_SERVE_LFS_SSH_ENABLED=%t", c.LFS.SSHEnabled), - fmt.Sprintf("SOFT_SERVE_JOBS_MIRROR_PULL=%s", c.Jobs.MirrorPull), + fmt.Sprintf("SOFT_SERVE_JOBS_MIRROR_PULL_ENABLED=%t", c.Jobs.MirrorPull.Enabled), + fmt.Sprintf("SOFT_SERVE_JOBS_MIRROR_PULL_SCHEDULE=%s", c.Jobs.MirrorPull.Schedule), + fmt.Sprintf("SOFT_SERVE_ALLOW_PUBLIC_GO_GET=%t", c.AllowPublicGoGet), }...) return envs @@ -350,10 +416,11 @@ func DefaultConfig() *Config { Name: "Soft Serve", DataPath: DefaultDataPath(), SSH: SSHConfig{ - Enabled: true, - ListenAddr: ":23231", - PublicURL: "ssh://localhost:23231", - KeyPath: filepath.Join("ssh", "soft_serve_host_ed25519"), + Enabled: true, + ListenAddr: ":23231", + PublicURL: "ssh://localhost:23231", + KeyPath: filepath.Join("ssh", "soft_serve_host_ed25519"), + AllowMouseEvents: true, ClientKeyPath: filepath.Join("ssh", "soft_serve_client_ed25519"), MaxTimeout: 0, IdleTimeout: 10 * 60, // 10 minutes @@ -370,6 +437,8 @@ func DefaultConfig() *Config { Enabled: true, ListenAddr: ":23232", PublicURL: "http://localhost:23232", + RateLimit: 10, + RateBurst: 30, CORS: CORSConfig{ AllowedHeaders: []string{"Accept", "Accept-Language", "Content-Language", "Content-Type", "Origin", "X-Requested-With", "User-Agent", "Authorization", "Access-Control-Request-Method", "Access-Control-Allow-Origin"}, AllowedMethods: []string{"GET", "HEAD", "POST", "PUT", "OPTIONS"}, @@ -394,7 +463,10 @@ func DefaultConfig() *Config { SSHEnabled: false, }, Jobs: JobsConfig{ - MirrorPull: "@every 10m", + MirrorPull: MirrorPullJobConfig{ + Enabled: true, + Schedule: "@every 10m", + }, }, } } @@ -443,7 +515,24 @@ func (c *Config) Validate() error { c.InitialAdminKeys = pks - c.HTTP.CORS.AllowedOrigins = append([]string{c.HTTP.PublicURL}, c.HTTP.CORS.AllowedOrigins...) + if c.AnonAccess != "" { + level := access.ParseAccessLevel(c.AnonAccess) + // ParseAccessLevel returns NoAccess for unknown strings, but "no-access" + // is also a valid explicit value, so check by re-serialising. + if level.String() != c.AnonAccess { + return fmt.Errorf("invalid anon_access %q: must be one of %s, %s, %s, %s", + c.AnonAccess, + access.NoAccess.String(), + access.ReadOnlyAccess.String(), + access.ReadWriteAccess.String(), + access.AdminAccess.String(), + ) + } + } + + if len(c.HTTP.CORS.AllowedOrigins) == 0 || c.HTTP.CORS.AllowedOrigins[0] != c.HTTP.PublicURL { + c.HTTP.CORS.AllowedOrigins = append([]string{c.HTTP.PublicURL}, c.HTTP.CORS.AllowedOrigins...) + } return nil } diff --git a/pkg/config/file.go b/pkg/config/file.go index 8170493c3..870facffa 100644 --- a/pkg/config/file.go +++ b/pkg/config/file.go @@ -48,6 +48,28 @@ ssh: # A value of 0 means no timeout. idle_timeout: {{ .SSH.IdleTimeout }} + # Allow mouse events in the TUI. When true (default), mouse clicks and + # scrolling work in the TUI. Set to false to restore terminal text selection. + # Can also be controlled with SOFT_SERVE_SSH_ALLOW_MOUSE_EVENTS=false. + allow_mouse_events: {{ .SSH.AllowMouseEvents }} + + # Allowed SSH key exchange algorithms. + # Leave empty to use the go/crypto defaults. + # key_exchanges: + # - curve25519-sha256 + # - ecdh-sha2-nistp256 + + # Allowed SSH ciphers. + # Leave empty to use the go/crypto defaults. + # ciphers: + # - aes128-gcm@openssh.com + # - chacha20-poly1305@openssh.com + + # Allowed SSH MAC algorithms. + # Leave empty to use the go/crypto defaults. + # macs: + # - hmac-sha2-256-etm@openssh.com + # The Git daemon configuration. git: # Enable the Git daemon. @@ -89,6 +111,20 @@ http: # Make sure to use https:// if you are using TLS. public_url: "{{ .HTTP.PublicURL }}" + # When true, repositories are accessible without the .git suffix in the URL. + # Both / and /.git will be accepted. + # strip_git_suffix: false + + # When true, the X-Forwarded-For header is trusted for client IP resolution. + # Only enable this when the server sits behind a trusted reverse proxy. + # trust_proxy_headers: false + + # Maximum HTTP requests per second per IP address. Set to 0 to disable. + # rate_limit: 10 + + # Maximum burst size for the HTTP rate limiter. + # rate_burst: 30 + # The cross-origin request security options cors: # The allowed cross-origin headers @@ -142,11 +178,30 @@ lfs: # Cron job configuration jobs: - mirror_pull: "{{ .Jobs.MirrorPull }}" + mirror_pull: + # Enable the periodic mirror pull job. + enabled: {{ .Jobs.MirrorPull.Enabled }} + # Cron schedule for the mirror pull job. + schedule: "{{ .Jobs.MirrorPull.Schedule }}" # Additional admin keys. #initial_admin_keys: # - "ssh-rsa AAAAB3NzaC1yc2..." + +# Anonymous access level applied on every startup. +# Overrides the value stored in the database when set. +# Valid values: no-access, read-only, read-write, admin-access. +# Leave commented out to preserve the database value. +#anon_access: read-only + +# Whether keyless (keyboard-interactive) access is allowed. +# Overrides the value stored in the database when set. +# Leave commented out to preserve the database value. +#allow_keyless: true + +# When true, serve go-get meta tags for private/hidden repositories. +# The actual git content remains inaccessible without credentials. +# allow_public_go_get: false `)) func newConfigFile(cfg *Config) string { diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index f37ae5549..0d98ae009 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -17,10 +17,12 @@ import ( "github.com/charmbracelet/soft-serve/pkg/backend" "github.com/charmbracelet/soft-serve/pkg/config" "github.com/charmbracelet/soft-serve/pkg/git" + "github.com/charmbracelet/soft-serve/pkg/ratelimit" "github.com/charmbracelet/soft-serve/pkg/utils" "github.com/go-git/go-git/v5/plumbing/format/pktline" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "golang.org/x/time/rate" ) var ( @@ -56,6 +58,8 @@ type GitDaemon struct { done atomic.Bool // indicates if the server has been closed listeners []net.Listener liMu sync.Mutex + limiter *ratelimit.IPLimiter + ipConns sync.Map // map[string]*int32 — active connections per IP } // NewGitDaemon returns a new Git daemon. @@ -71,6 +75,7 @@ func NewGitDaemon(ctx context.Context) (*GitDaemon, error) { conns: connections{m: make(map[net.Conn]struct{})}, logger: log.FromContext(ctx).WithPrefix("gitdaemon"), } + d.limiter = ratelimit.New(rate.Limit(5), 10, 5*time.Minute) return d, nil } @@ -99,7 +104,6 @@ func (d *GitDaemon) Serve(listener net.Listener) error { d.listeners = append(d.listeners, listener) d.liMu.Unlock() - var tempDelay time.Duration for { conn, err := listener.Accept() if err != nil { @@ -109,32 +113,51 @@ func (d *GitDaemon) Serve(listener net.Listener) error { default: d.logger.Debugf("git: error accepting connection: %v", err) } - if ne, ok := err.(net.Error); ok && ne.Temporary() { - if tempDelay == 0 { - tempDelay = 5 * time.Millisecond - } else { - tempDelay *= 2 - } - if max := 1 * time.Second; tempDelay > max { //nolint:revive - tempDelay = max - } - time.Sleep(tempDelay) - continue - } return err } - // Close connection if there are too many open connections. - if d.conns.Size()+1 >= d.cfg.Git.MaxConnections { + // Close connection if we are already at the configured limit. + // Using >= (not >) ensures MaxConnections is the inclusive cap. + if d.conns.Size() >= d.cfg.Git.MaxConnections { d.logger.Debugf("git: max connections reached, closing %s", conn.RemoteAddr()) d.fatal(conn, git.ErrMaxConnections) continue } + // Per-IP connection cap: reject IPs with >= 10 concurrent connections. + const maxConnsPerIP = 10 + remoteIP, _, _ := net.SplitHostPort(conn.RemoteAddr().String()) + if remoteIP == "" { + remoteIP = conn.RemoteAddr().String() + } + val, _ := d.ipConns.LoadOrStore(remoteIP, new(int32)) + ipCount := val.(*int32) + if atomic.AddInt32(ipCount, 1) > maxConnsPerIP { + atomic.AddInt32(ipCount, -1) + d.logger.Debugf("git: per-IP connection limit reached, closing %s", conn.RemoteAddr()) + d.fatal(conn, git.ErrMaxConnections) + continue + } + d.wg.Add(1) go func() { + defer d.wg.Done() + defer func() { + // Decrement the per-IP counter and remove the map entry when it + // reaches zero so ipConns does not grow without bound over time. + // CompareAndDelete is a no-op if another connection from the same IP + // already stored a new counter pointer via LoadOrStore. + // + // Known limitation: there is a brief window between AddInt32 + // returning 0 and CompareAndDelete completing. A new connection in + // that window reuses the stale pointer and may find its map entry + // deleted, temporarily bypassing the per-IP cap. The global + // d.conns limit still guards against DoS. + if atomic.AddInt32(ipCount, -1) == 0 { + d.ipConns.CompareAndDelete(remoteIP, ipCount) + } + }() d.handleClient(conn) - d.wg.Done() }() } } @@ -146,9 +169,27 @@ func (d *GitDaemon) fatal(c net.Conn, err error) { } } +// sanitizeParamValue rejects values containing newlines, nulls, or other +// control characters that could corrupt the GIT_PROTOCOL env var. +func sanitizeParamValue(s string) (string, bool) { + for _, r := range s { + if r < 0x20 || r == 0x7f { + return "", false + } + } + return s, true +} + // handleClient handles a git protocol client. func (d *GitDaemon) handleClient(conn net.Conn) { - ctx, cancel := context.WithCancel(context.Background()) + ip := conn.RemoteAddr().String() + if !d.limiter.Allow(ip) { + d.logger.Warn("git daemon rate limited", "remote", ip) + conn.Close() //nolint: errcheck + return + } + + ctx, cancel := context.WithCancel(d.ctx) idleTimeout := time.Duration(d.cfg.Git.IdleTimeout) * time.Second c := &serverConn{ Conn: conn, @@ -218,7 +259,15 @@ func (d *GitDaemon) handleClient(conn net.Conn) { return } - host := strings.TrimPrefix(string(opts[1]), "host=") + rawHost := strings.TrimPrefix(string(opts[1]), "host=") + // Sanitize host value for control characters before it enters the + // process environment (newlines in env blocks corrupt other vars). + host, hostOK := sanitizeParamValue(rawHost) + if !hostOK { + d.logger.Warnf("git: rejecting connection with invalid host value %q", rawHost) + d.fatal(c, git.ErrInvalidRequest) + return + } extraParams := map[string]string{} if len(opts) > 2 { @@ -265,7 +314,10 @@ func (d *GitDaemon) handleClient(conn net.Conn) { } if _, err := d.be.Repository(ctx, repo); err != nil { - d.fatal(c, git.ErrInvalidRepo) + // Return ErrNotAuthed (not ErrInvalidRepo) so that the response is + // indistinguishable from an access-denial. Returning ErrInvalidRepo + // would let unauthenticated clients enumerate which repositories exist. + d.fatal(c, git.ErrNotAuthed) return } @@ -284,15 +336,29 @@ func (d *GitDaemon) handleClient(conn net.Conn) { } // Add git protocol environment variable. + // Only the "version" key is accepted; values are checked for control characters. if len(extraParams) > 0 { var gitProto string for k, v := range extraParams { + if k != "version" { + d.logger.Warnf("git: ignoring unknown extra param key %q", k) + continue + } + // k is already confirmed to be "version" (ASCII-safe); only + // the value needs sanitization for control-character injection. + sv, ok := sanitizeParamValue(v) + if !ok { + d.logger.Warnf("git: dropping extra param with unsafe value for key %q", k) + continue + } if len(gitProto) > 0 { gitProto += ":" } - gitProto += k + "=" + v + gitProto += k + "=" + sv + } + if gitProto != "" { + envs = append(envs, "GIT_PROTOCOL="+gitProto) } - envs = append(envs, "GIT_PROTOCOL="+gitProto) } envs = append(envs, d.cfg.Environ()...) @@ -311,7 +377,7 @@ func (d *GitDaemon) handleClient(conn net.Conn) { return } - counter.WithLabelValues(name) + counter.WithLabelValues(name).Inc() } } @@ -327,11 +393,11 @@ func (d *GitDaemon) closeListener() error { if d.done.Load() { return ErrServerClosed } - var err error + var errs []error d.liMu.Lock() for _, l := range d.listeners { - if err = l.Close(); err != nil { - err = errors.Join(err, fmt.Errorf("close listener %s: %w", l.Addr(), err)) + if err := l.Close(); err != nil { + errs = append(errs, fmt.Errorf("close listener %s: %w", l.Addr(), err)) } } d.listeners = d.listeners[:0] @@ -340,7 +406,7 @@ func (d *GitDaemon) closeListener() error { d.done.Store(true) close(d.finished) }) - return err + return errors.Join(errs...) } // Shutdown gracefully shuts down the daemon. diff --git a/pkg/daemon/daemon_test.go b/pkg/daemon/daemon_test.go index dc8dafd93..fed07418a 100644 --- a/pkg/daemon/daemon_test.go +++ b/pkg/daemon/daemon_test.go @@ -102,8 +102,10 @@ func TestInvalidRepo(t *testing.T) { t.Fatalf("expected nil, got error: %v", err) } _, err = readPktline(c) - if err != nil && err.Error() != git.ErrInvalidRepo.Error() { - t.Errorf("expected %q error, got %q", git.ErrInvalidRepo, err) + // Non-existent repos now return ErrNotAuthed (same as access-denied) to + // prevent unauthenticated clients from enumerating repository existence. + if err != nil && err.Error() != git.ErrNotAuthed.Error() { + t.Errorf("expected %q error, got %q", git.ErrNotAuthed, err) } } diff --git a/pkg/db/db.go b/pkg/db/db.go index 2d27ae0ec..e5f730776 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "strings" "charm.land/log/v2" "github.com/charmbracelet/soft-serve/pkg/config" @@ -26,6 +27,12 @@ func Open(ctx context.Context, driverName string, dsn string) (*DB, error) { return nil, err } + if strings.HasPrefix(driverName, "sqlite") { + if _, err := db.ExecContext(ctx, "PRAGMA journal_mode=WAL"); err != nil { + return nil, fmt.Errorf("enable WAL mode: %w", err) + } + } + d := &DB{ DB: db, } diff --git a/pkg/db/migrate/0004_push_mirrors.go b/pkg/db/migrate/0004_push_mirrors.go new file mode 100644 index 000000000..11048bb53 --- /dev/null +++ b/pkg/db/migrate/0004_push_mirrors.go @@ -0,0 +1,23 @@ +package migrate + +import ( + "context" + + "github.com/charmbracelet/soft-serve/pkg/db" +) + +const ( + pushMirrorsName = "push_mirrors" + pushMirrorsVersion = 4 +) + +var pushMirrors = Migration{ + Name: pushMirrorsName, + Version: pushMirrorsVersion, + Migrate: func(ctx context.Context, tx *db.Tx) error { + return migrateUp(ctx, tx, pushMirrorsVersion, pushMirrorsName) + }, + Rollback: func(ctx context.Context, tx *db.Tx) error { + return migrateDown(ctx, tx, pushMirrorsVersion, pushMirrorsName) + }, +} diff --git a/pkg/db/migrate/0004_push_mirrors_postgres.down.sql b/pkg/db/migrate/0004_push_mirrors_postgres.down.sql new file mode 100644 index 000000000..1e2391344 --- /dev/null +++ b/pkg/db/migrate/0004_push_mirrors_postgres.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS push_mirrors; diff --git a/pkg/db/migrate/0004_push_mirrors_postgres.up.sql b/pkg/db/migrate/0004_push_mirrors_postgres.up.sql new file mode 100644 index 000000000..41c173acc --- /dev/null +++ b/pkg/db/migrate/0004_push_mirrors_postgres.up.sql @@ -0,0 +1,10 @@ +CREATE TABLE IF NOT EXISTS push_mirrors ( + id BIGSERIAL PRIMARY KEY, + repo_id BIGINT NOT NULL REFERENCES repos(id) ON DELETE CASCADE, + name TEXT NOT NULL, + remote_url TEXT NOT NULL, + enabled BOOLEAN NOT NULL DEFAULT TRUE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + UNIQUE(repo_id, name) +); diff --git a/pkg/db/migrate/0004_push_mirrors_sqlite.down.sql b/pkg/db/migrate/0004_push_mirrors_sqlite.down.sql new file mode 100644 index 000000000..1e2391344 --- /dev/null +++ b/pkg/db/migrate/0004_push_mirrors_sqlite.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS push_mirrors; diff --git a/pkg/db/migrate/0004_push_mirrors_sqlite.up.sql b/pkg/db/migrate/0004_push_mirrors_sqlite.up.sql new file mode 100644 index 000000000..b5142caba --- /dev/null +++ b/pkg/db/migrate/0004_push_mirrors_sqlite.up.sql @@ -0,0 +1,10 @@ +CREATE TABLE IF NOT EXISTS push_mirrors ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + repo_id INTEGER NOT NULL REFERENCES repos(id) ON DELETE CASCADE, + name TEXT NOT NULL, + remote_url TEXT NOT NULL, + enabled BOOLEAN NOT NULL DEFAULT TRUE, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + UNIQUE(repo_id, name) +); diff --git a/pkg/db/migrate/migrations.go b/pkg/db/migrate/migrations.go index d134f2304..7e5a27806 100644 --- a/pkg/db/migrate/migrations.go +++ b/pkg/db/migrate/migrations.go @@ -18,6 +18,7 @@ var migrations = []Migration{ createTables, webhooks, migrateLfsObjects, + pushMirrors, } func execMigration(ctx context.Context, tx *db.Tx, version int, name string, down bool) error { diff --git a/pkg/db/models/push_mirror.go b/pkg/db/models/push_mirror.go new file mode 100644 index 000000000..e549b5113 --- /dev/null +++ b/pkg/db/models/push_mirror.go @@ -0,0 +1,14 @@ +package models + +import "time" + +// PushMirror represents a push mirror for a repository. +type PushMirror struct { + ID int64 `db:"id"` + RepoID int64 `db:"repo_id"` + Name string `db:"name"` + RemoteURL string `db:"remote_url"` + Enabled bool `db:"enabled"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` +} diff --git a/pkg/git/git.go b/pkg/git/git.go index 5ad5998be..a6b4c8b0f 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -51,7 +51,7 @@ func EnsureWithin(reposDir string, repo string) error { } // ensure the repo is within the repos directory - if !strings.HasPrefix(absRepo, absRepos) { + if !strings.HasPrefix(absRepo, absRepos+string(filepath.Separator)) && absRepo != absRepos { log.Debugf("repo path is outside of repos directory: %s", absRepo) return ErrInvalidRepo } diff --git a/pkg/git/lfs.go b/pkg/git/lfs.go index 14846c120..801f2e8a8 100644 --- a/pkg/git/lfs.go +++ b/pkg/git/lfs.go @@ -147,12 +147,6 @@ func (t *lfsTransfer) Upload(oid string, size int64, r io.Reader, _ transfer.Arg return err } - obj, err := t.storage.Open(tempName) - if err != nil { - t.logger.Errorf("error opening object: %v", err) - return err - } - pointer := transfer.Pointer{ Oid: oid, } @@ -167,7 +161,7 @@ func (t *lfsTransfer) Upload(oid string, size int64, r io.Reader, _ transfer.Arg } expectedPath := path.Join("objects", pointer.RelativePath()) - if err := t.storage.Rename(obj.Name(), expectedPath); err != nil { + if err := t.storage.Rename(tempName, expectedPath); err != nil { t.logger.Errorf("error renaming object: %v", err) _ = t.store.DeleteLFSObjectByOid(t.ctx, t.dbx, t.repo.ID(), pointer.Oid) return err diff --git a/pkg/git/lfs_auth.go b/pkg/git/lfs_auth.go index b2175cf3d..6d3270863 100644 --- a/pkg/git/lfs_auth.go +++ b/pkg/git/lfs_auth.go @@ -54,7 +54,7 @@ func LFSAuthenticate(ctx context.Context, cmd ServiceCommand) error { expiresAt := now.Add(expiresIn) claims := jwt.RegisteredClaims{ Subject: fmt.Sprintf("%s#%d", user.Username(), user.ID()), - ExpiresAt: jwt.NewNumericDate(expiresAt), // expire in an hour + ExpiresAt: jwt.NewNumericDate(expiresAt), NotBefore: jwt.NewNumericDate(now), IssuedAt: jwt.NewNumericDate(now), Issuer: cfg.HTTP.PublicURL, @@ -72,7 +72,7 @@ func LFSAuthenticate(ctx context.Context, cmd ServiceCommand) error { } href := fmt.Sprintf("%s/%s.git/info/lfs", cfg.HTTP.PublicURL, repo.Name()) - logger.Debug("generated token", "token", j, "href", href, "expires_at", expiresAt) + logger.Debug("generated token", "href", href, "expires_at", expiresAt) return json.NewEncoder(cmd.Stdout).Encode(lfs.AuthenticateResponse{ Header: map[string]string{ @@ -80,6 +80,6 @@ func LFSAuthenticate(ctx context.Context, cmd ServiceCommand) error { }, Href: href, ExpiresAt: expiresAt, - ExpiresIn: expiresIn, + ExpiresIn: int64(expiresIn / time.Second), }) } diff --git a/pkg/git/service.go b/pkg/git/service.go index a4066d07e..11769cfaa 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -9,6 +9,7 @@ import ( "os/exec" "strings" "sync" + "time" "charm.land/log/v2" ) @@ -57,8 +58,27 @@ func (s Service) Handler(ctx context.Context, cmd ServiceCommand) error { type ServiceHandler func(ctx context.Context, cmd ServiceCommand) error // gitServiceHandler is the default service handler using the git binary. +// +// Deadline invariant: the ctx passed here must be cancellation-only — it must +// NOT carry a context.WithDeadline or context.WithTimeout. All three transport +// callers (SSH via gliderlabs/ssh, git daemon via pkg/daemon, HTTP via +// net/http) enforce timeouts by calling conn.SetDeadline on the underlying +// net.Conn, NOT by attaching a deadline to the context. If a caller were ever +// changed to pass a deadline-carrying context, exec.CommandContext would kill +// the git subprocess when the deadline expires, potentially corrupting an +// in-progress push. In that case, replace ctx with a context.WithCancelCause +// derived from context.Background() that mirrors parent cancellation but not +// DeadlineExceeded. func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) error { + // NOTE: ctx is cancellation-only (derived from context.Background via + // context.WithCancel). SSH and git-daemon timeouts fire as net.Conn + // deadline errors, not context deadlines — so no deadline can reach here + // and kill a long-running push or clone mid-transfer. cmd := exec.CommandContext(ctx, "git") + // WaitDelay bounds how long we wait for stdin/stdout pipe goroutines to + // finish after the git process has already been killed (e.g. by context + // cancellation). It does NOT impose a timeout on running git operations. + cmd.WaitDelay = 30 * time.Second cmd.Dir = scmd.Dir cmd.Args = append(cmd.Args, []string{ // Enable partial clones @@ -75,10 +95,16 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er cmd.Args = append(cmd.Args, ".") - cmd.Env = os.Environ() - if len(scmd.Env) > 0 { - cmd.Env = append(cmd.Env, scmd.Env...) + // Build a minimal environment for the git subprocess. + // Start with only HOME and PATH; callers append SOFT_SERVE_* vars via scmd.Env. + minimal := []string{ + "HOME=" + os.Getenv("HOME"), + "PATH=" + os.Getenv("PATH"), } + if v := os.Getenv("GIT_EXEC_PATH"); v != "" { + minimal = append(minimal, "GIT_EXEC_PATH="+v) + } + cmd.Env = append(minimal, scmd.Env...) if scmd.CmdFunc != nil { scmd.CmdFunc(cmd) @@ -125,8 +151,11 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er if scmd.Stdin != nil { go func() { defer stdin.Close() //nolint: errcheck - if _, err := io.Copy(stdin, scmd.Stdin); err != nil { - log.Errorf("gitServiceHandler: failed to copy stdin: %v", err) + if _, err := io.Copy(stdin, scmd.Stdin); err != nil && ctx.Err() == nil { + // Broken-pipe here is normal: git read all it needed and exited, + // cmd.Wait() closed the write end of the pipe before the client + // finished sending. Log at Debug to avoid spurious error noise. + log.FromContext(ctx).Debug("gitServiceHandler: stdin copy ended", "err", err) } }() } @@ -136,7 +165,7 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er wg.Add(1) go func() { defer wg.Done() - if _, err := io.Copy(scmd.Stdout, stdout); err != nil { + if _, err := io.Copy(scmd.Stdout, stdout); err != nil && ctx.Err() == nil { log.Errorf("gitServiceHandler: failed to copy stdout: %v", err) } }() @@ -147,7 +176,7 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er wg.Add(1) go func() { defer wg.Done() - if _, erro := io.Copy(scmd.Stderr, stderr); err != nil { + if _, erro := io.Copy(scmd.Stderr, stderr); erro != nil && ctx.Err() == nil { log.Errorf("gitServiceHandler: failed to copy stderr: %v", erro) } }() @@ -159,12 +188,39 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er wg.Wait() err = cmd.Wait() - if err != nil && errors.Is(err, os.ErrNotExist) { - return ErrInvalidRepo - } else if err != nil { + if err != nil { var exitErr *exec.ExitError - if errors.As(err, &exitErr) && len(exitErr.Stderr) > 0 { - return fmt.Errorf("%s: %s", exitErr, exitErr.Stderr) + // Note: errors.As correctly unwraps through errors.Join, which Go 1.20+ + // uses when cmd.WaitDelay fires (joining the ExitError with a timeout + // error). Verified: errors.As(errors.Join(exitErr, timeoutErr), &exitErr) + // returns true. So the suppression path below is safe even when WaitDelay + // wraps the ExitError via errors.Join. + if errors.As(err, &exitErr) { + if errors.Is(ctx.Err(), context.Canceled) { + // Process exited because context was cancelled — client disconnected + // or server is shutting down. Expected; not worth surfacing. + // We do not gate on ExitCode()==-1: on Unix a signal-killed process + // has no exit code (-1), but on Windows TerminateProcess sets exit + // code 1. Checking ctx.Err() alone is portable across both. + log.FromContext(ctx).Debug("git process exited on context cancellation", "service", svc.Name()) + return nil + } + // When WaitDelay fires alongside a non-zero exit, cmd.Wait returns + // errors.Join(exitErr, exec.ErrWaitDelay). Strip the WaitDelay noise + // so callers see a clean exit-status error, not an internal timeout. + retErr := error(exitErr) + if len(exitErr.Stderr) > 0 { + retErr = fmt.Errorf("%w: %s", exitErr, exitErr.Stderr) + } + return retErr + } else if errors.Is(err, exec.ErrWaitDelay) { + // WaitDelay (30s) expired before pipe goroutines drained. This branch + // is only reached when there is no accompanying ExitError (i.e. git + // exited 0 but the client read the pipe slowly). The git command + // succeeded; the drain timeout is an internal bookkeeping detail, not + // a caller-visible error. + log.FromContext(ctx).Debug("git pipe drain timed out", "service", svc.Name()) + return nil } return err diff --git a/pkg/hooks/gen.go b/pkg/hooks/gen.go index 24415a788..9173c871f 100644 --- a/pkg/hooks/gen.go +++ b/pkg/hooks/gen.go @@ -3,11 +3,11 @@ package hooks import ( "bytes" "context" + "errors" "os" "path/filepath" "text/template" - "charm.land/log/v2" "github.com/charmbracelet/soft-serve/pkg/config" "github.com/charmbracelet/soft-serve/pkg/utils" ) @@ -27,7 +27,9 @@ const ( // - post-update // // This function should be called by the backend when a repository is created. -// TODO: support context. +// The context parameter is currently unused but is reserved for future use +// (e.g. cancellation during slow filesystem operations, operation-scoped logging). +// TODO: wire context into hook execution if/when implementing async hooks. func GenerateHooks(_ context.Context, cfg *config.Config, repo string) error { repo = utils.SanitizeRepo(repo) + ".git" hooksPath := filepath.Join(cfg.DataPath, "repos", repo, "hooks") @@ -35,6 +37,7 @@ func GenerateHooks(_ context.Context, cfg *config.Config, repo string) error { return err } + var errs []error for _, hook := range []string{ PreReceiveHook, UpdateHook, @@ -49,20 +52,22 @@ func GenerateHooks(_ context.Context, cfg *config.Config, repo string) error { // Write the hooks primary script if err := os.WriteFile(hp, []byte(hookTemplate), os.ModePerm); err != nil { //nolint:gosec - return err + errs = append(errs, err) + continue } // Create ${hook}.d directory. hp += ".d" if err := os.MkdirAll(hp, os.ModePerm); err != nil { - return err + errs = append(errs, err) + continue } switch hook { case UpdateHook: - args = "$1 $2 $3" + args = `"$1" "$2" "$3"` case PostUpdateHook: - args = "$@" + args = `"$@"` } if err := hooksTmpl.Execute(&data, struct { @@ -74,20 +79,18 @@ func GenerateHooks(_ context.Context, cfg *config.Config, repo string) error { Hook: hook, Args: args, }); err != nil { - log.WithPrefix("hooks").Error("failed to execute hook template", "err", err) + errs = append(errs, err) continue } // Write the soft-serve hook inside ${hook}.d directory. hp = filepath.Join(hp, "soft-serve") - err := os.WriteFile(hp, data.Bytes(), os.ModePerm) //nolint:gosec - if err != nil { - log.WithPrefix("hooks").Error("failed to write hook", "err", err) - continue + if err := os.WriteFile(hp, data.Bytes(), os.ModePerm); err != nil { //nolint:gosec + errs = append(errs, err) } } - return nil + return errors.Join(errs...) } const ( @@ -97,14 +100,16 @@ const ( # AUTO GENERATED BY SOFT SERVE, DO NOT MODIFY data=$(cat) exitcodes="" -hookname=$(basename $0) -GIT_DIR=${GIT_DIR:-$(dirname $0)/..} -for hook in ${GIT_DIR}/hooks/${hookname}.d/*; do +hookname=$(basename "$0") +# $0 is set by git to the path of the hook script itself — not from user input. +# The command substitution $(dirname "$0") is therefore safe. +GIT_DIR=${GIT_DIR:-$(dirname "$0")/..} +for hook in "${GIT_DIR}/hooks/${hookname}.d/"*; do # Avoid running non-executable hooks test -x "${hook}" && test -f "${hook}" || continue # Run the actual hook - echo "${data}" | "${hook}" "$@" + printf '%s' "${data}" | "${hook}" "$@" # Store the exit code for later use exitcodes="${exitcodes} $?" @@ -117,8 +122,10 @@ done ` ) -// hooksTmpl is the soft-serve hook that will be run by the git hooks -// inside the hooks directory. +// hooksTmpl is the soft-serve hook that will be run by the git hooks inside +// the hooks directory. Template values are server-controlled (binary path, +// hook name constants, and hardcoded argument strings) — not user input — +// so no shell escaping of template substitutions is required. var hooksTmpl = template.Must(template.New("hooks").Parse(`#!/usr/bin/env bash # AUTO GENERATED BY SOFT SERVE, DO NOT MODIFY if [ -z "$SOFT_SERVE_REPO_NAME" ]; then diff --git a/pkg/jobs/mirror.go b/pkg/jobs/mirror.go index 4642e72b1..fb6c0448d 100644 --- a/pkg/jobs/mirror.go +++ b/pkg/jobs/mirror.go @@ -6,6 +6,7 @@ import ( "path/filepath" "runtime" "strings" + "time" "charm.land/log/v2" "github.com/charmbracelet/soft-serve/git" @@ -21,15 +22,22 @@ func init() { Register("mirror-pull", mirrorPull{}) } +const mirrorPullTimeout = 30 * time.Minute + type mirrorPull struct{} // Spec derives the spec used for pull mirrors and implements Runner. +// Returns an empty string when the job is disabled, which causes the scheduler +// to skip registering it. func (m mirrorPull) Spec(ctx context.Context) string { cfg := config.FromContext(ctx) - if cfg.Jobs.MirrorPull != "" { - return cfg.Jobs.MirrorPull + if !cfg.Jobs.MirrorPull.Enabled { + return "" + } + if cfg.Jobs.MirrorPull.Schedule == "" { + return "@every 10m" } - return "@every 10m" + return cfg.Jobs.MirrorPull.Schedule } // Func runs the (pull) mirror job task and implements Runner. @@ -69,20 +77,38 @@ func (m mirrorPull) Func(ctx context.Context) func() { "remote update --prune", // update remote and prune remote refs } + // Build the SSH env once; reused for every git command below. + sshEnv := fmt.Sprintf(`GIT_SSH_COMMAND=ssh -o UserKnownHostsFile="%s" -o StrictHostKeyChecking=accept-new -i "%s"`, + filepath.Join(cfg.DataPath, "ssh", "known_hosts"), + cfg.SSH.ClientKeyPath, + ) + + syncOK := true for _, c := range cmds { args := strings.Split(c, " ") - cmd := git.NewCommand(args...).WithContext(ctx) - cmd.AddEnvs( - fmt.Sprintf(`GIT_SSH_COMMAND=ssh -o UserKnownHostsFile="%s" -o StrictHostKeyChecking=no -i "%s"`, - filepath.Join(cfg.DataPath, "ssh", "known_hosts"), - cfg.SSH.ClientKeyPath, - ), - ) + // Prepend -c gc.auto=0 to disable automatic garbage + // collection. git fetch and git remote update both + // trigger gc --auto by default; on large repos this + // spawns git rev-list --objects --all and pegs a CPU + // for the entire sync window. + args = append([]string{"-c", "gc.auto=0"}, args...) + // Use no timeout (-1) so that syncing large repos is not + // killed by the 1-minute DefaultTimeout. The initial clone + // in ImportRepository already sets Timeout: -1 for the + // same reason. git-module RunInDirWithOptions only applies + // a deadline when timeout > 0, so -1 is safe here. + cmd := git.NewCommand(args...).WithContext(ctx).WithTimeout(mirrorPullTimeout) + cmd.AddEnvs(sshEnv) if _, err := cmd.RunInDir(r.Path); err != nil { - logger.Error("error running git remote update", "repo", name, "err", err) + logger.Error("error running git command", "cmd", c, "repo", name, "err", err) + syncOK = false + break } } + if !syncOK { + return + } if cfg.LFS.Enabled { rcfg, err := r.Config() diff --git a/pkg/jwk/jwk.go b/pkg/jwk/jwk.go index f7fe92404..43d8dd901 100644 --- a/pkg/jwk/jwk.go +++ b/pkg/jwk/jwk.go @@ -2,6 +2,7 @@ package jwk import ( "crypto" + "crypto/ed25519" "crypto/sha256" "fmt" @@ -37,8 +38,14 @@ func NewPair(cfg *config.Config) (Pair, error) { return Pair{}, err } - sum := sha256.Sum256(kp.RawPrivateKey()) - kid := fmt.Sprintf("%x", sum) + // Derive kid from the public key bytes (Ed25519 public key is a []byte). + var kidBytes []byte + if pub, ok := kp.CryptoPublicKey().(ed25519.PublicKey); ok { + kidBytes = pub + } else { + kidBytes = kp.RawPrivateKey() + } + kid := fmt.Sprintf("%x", sha256.Sum256(kidBytes)) jwk := jose.JSONWebKey{ Key: kp.CryptoPublicKey(), KeyID: kid, diff --git a/pkg/lfs/common.go b/pkg/lfs/common.go index aa98c6527..3162f640b 100644 --- a/pkg/lfs/common.go +++ b/pkg/lfs/common.go @@ -68,7 +68,12 @@ type Link struct { Href string `json:"href"` Header map[string]string `json:"header,omitempty"` ExpiresAt *time.Time `json:"expires_at,omitempty"` - ExpiresIn *time.Duration `json:"expires_in,omitempty"` + // ExpiresIn is the number of seconds (not nanoseconds, not milliseconds) + // until the link expires, per the git-lfs batch API spec. Callers MUST + // pass whole seconds (e.g. int64(d / time.Second)). Using int64 instead + // of time.Duration avoids serialising nanoseconds, which overflows int32 + // on 32-bit platforms. + ExpiresIn *int64 `json:"expires_in,omitempty"` } // ObjectError defines the JSON structure returned to the client in case of an error. @@ -94,10 +99,14 @@ type Reference struct { } // AuthenticateResponse is the git-lfs-authenticate JSON response object. +// ExpiresIn is encoded as seconds per the git-lfs SSH authentication spec: +// https://github.com/git-lfs/git-lfs/blob/main/docs/api/server-discovery.md +// Using int64 (not time.Duration) avoids serialising nanoseconds, which +// overflows int32 on 32-bit platforms (issue #781). type AuthenticateResponse struct { Header map[string]string `json:"header"` Href string `json:"href"` - ExpiresIn time.Duration `json:"expires_in"` + ExpiresIn int64 `json:"expires_in"` ExpiresAt time.Time `json:"expires_at"` } diff --git a/pkg/lfs/scanner.go b/pkg/lfs/scanner.go index 6688e4225..b17cbd496 100644 --- a/pkg/lfs/scanner.go +++ b/pkg/lfs/scanner.go @@ -108,7 +108,6 @@ func catFileBatch(ctx context.Context, shasToBatchReader *io.PipeReader, catFile defer catFileBatchWriter.Close() //nolint: errcheck stderr := new(bytes.Buffer) - var errbuf strings.Builder if err := gitm.NewCommandWithContext(ctx, "cat-file", "--batch"). WithTimeout(-1). RunInDirWithOptions(basePath, gitm.RunInDirOptions{ @@ -116,7 +115,7 @@ func catFileBatch(ctx context.Context, shasToBatchReader *io.PipeReader, catFile Stdin: shasToBatchReader, Stderr: stderr, }); err != nil { - _ = shasToBatchReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %w - %s", basePath, err, errbuf.String())) + _ = shasToBatchReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %w - %s", basePath, err, stderr.String())) } } @@ -158,7 +157,6 @@ func catFileBatchCheck(ctx context.Context, shasToCheckReader *io.PipeReader, ca defer catFileCheckWriter.Close() //nolint: errcheck stderr := new(bytes.Buffer) - var errbuf strings.Builder if err := gitm.NewCommandWithContext(ctx, "cat-file", "--batch-check"). WithTimeout(-1). RunInDirWithOptions(basePath, gitm.RunInDirOptions{ @@ -166,7 +164,7 @@ func catFileBatchCheck(ctx context.Context, shasToCheckReader *io.PipeReader, ca Stdin: shasToCheckReader, Stderr: stderr, }); err != nil { - _ = shasToCheckReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %w - %s", basePath, err, errbuf.String())) + _ = shasToCheckReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %w - %s", basePath, err, stderr.String())) } } @@ -204,13 +202,12 @@ func revListAllObjects(ctx context.Context, revListWriter *io.PipeWriter, wg *sy defer revListWriter.Close() //nolint: errcheck stderr := new(bytes.Buffer) - var errbuf strings.Builder if err := gitm.NewCommandWithContext(ctx, "rev-list", "--objects", "--all"). WithTimeout(-1). RunInDirWithOptions(basePath, gitm.RunInDirOptions{ Stdout: revListWriter, Stderr: stderr, }); err != nil { - errChan <- fmt.Errorf("git rev-list [%s]: %w - %s", basePath, err, errbuf.String()) + errChan <- fmt.Errorf("git rev-list [%s]: %w - %s", basePath, err, stderr.String()) } } diff --git a/pkg/ratelimit/ratelimit.go b/pkg/ratelimit/ratelimit.go new file mode 100644 index 000000000..3dc078dcd --- /dev/null +++ b/pkg/ratelimit/ratelimit.go @@ -0,0 +1,91 @@ +// Package ratelimit provides a simple per-IP token-bucket rate limiter +// backed by golang.org/x/time/rate. It is intentionally minimal: one +// IPLimiter instance per server (SSH / HTTP / Git daemon) with a background +// goroutine that evicts stale entries. +package ratelimit + +import ( + "net" + "sync" + "time" + + "golang.org/x/time/rate" +) + +// IPLimiter tracks one token-bucket limiter per source IP address. +type IPLimiter struct { + mu sync.Mutex + entries map[string]*ipEntry + r rate.Limit + burst int + ttl time.Duration + done chan struct{} + closeOnce sync.Once +} + +type ipEntry struct { + lim *rate.Limiter + lastSeen time.Time +} + +// New creates an IPLimiter that allows r tokens/second with the given burst +// per source IP. Idle entries are evicted after ttl. +func New(r rate.Limit, burst int, ttl time.Duration) *IPLimiter { + il := &IPLimiter{ + entries: make(map[string]*ipEntry), + r: r, + burst: burst, + ttl: ttl, + done: make(chan struct{}), + } + go il.cleanup() + return il +} + +// Close stops the background cleanup goroutine. +func (il *IPLimiter) Close() { + il.closeOnce.Do(func() { close(il.done) }) +} + +// Allow returns true if the source IP is within its rate limit. +// ip may be in "host:port" form; the port is stripped automatically. +func (il *IPLimiter) Allow(ip string) bool { + if host, _, err := net.SplitHostPort(ip); err == nil { + ip = host + } + il.mu.Lock() + e, ok := il.entries[ip] + if !ok { + e = &ipEntry{lim: rate.NewLimiter(il.r, il.burst)} + il.entries[ip] = e + } + e.lastSeen = time.Now() + allow := e.lim.Allow() + il.mu.Unlock() + return allow +} + +func (il *IPLimiter) cleanup() { + cleanupInterval := il.ttl / 5 + // cleanupInterval is clamped to a minimum of 1 minute regardless of TTL to avoid + // excessive ticker overhead with very short TTLs. + if cleanupInterval < time.Minute { + cleanupInterval = time.Minute + } + ticker := time.NewTicker(cleanupInterval) + defer ticker.Stop() + for { + select { + case <-ticker.C: + il.mu.Lock() + for ip, e := range il.entries { + if time.Since(e.lastSeen) > il.ttl { + delete(il.entries, ip) + } + } + il.mu.Unlock() + case <-il.done: + return + } + } +} diff --git a/pkg/ssh/cmd/cmd.go b/pkg/ssh/cmd/cmd.go index 18624eb33..d82d4ebb9 100644 --- a/pkg/ssh/cmd/cmd.go +++ b/pkg/ssh/cmd/cmd.go @@ -1,6 +1,7 @@ package cmd import ( + "context" "fmt" "net/url" "strings" @@ -109,6 +110,17 @@ func CommandName(args []string) string { return args[0] } +// currentUser resolves the authenticated user from the session context. +// It checks the context user first (set by AuthenticationMiddleware for +// both public key and token auth), avoiding a redundant DB lookup. +func currentUser(ctx context.Context) (proto.User, error) { + user := proto.UserFromContext(ctx) + if user != nil { + return user, nil + } + return nil, proto.ErrUserNotFound +} + func checkIfReadable(cmd *cobra.Command, args []string) error { var repo string if len(args) > 0 { diff --git a/pkg/ssh/cmd/collab.go b/pkg/ssh/cmd/collab.go index df1e059f5..6c099d9d9 100644 --- a/pkg/ssh/cmd/collab.go +++ b/pkg/ssh/cmd/collab.go @@ -28,7 +28,7 @@ func collabAddCommand() *cobra.Command { Short: "Add a collaborator to a repo", Long: "Add a collaborator to a repo. LEVEL can be one of: no-access, read-only, read-write, or admin-access. Defaults to read-write.", Args: cobra.RangeArgs(2, 3), - PersistentPreRunE: checkIfReadableAndCollab, + PersistentPreRunE: checkIfAdmin, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) @@ -54,7 +54,7 @@ func collabRemoveCommand() *cobra.Command { Use: "remove REPOSITORY USERNAME", Args: cobra.ExactArgs(2), Short: "Remove a collaborator from a repo", - PersistentPreRunE: checkIfReadableAndCollab, + PersistentPreRunE: checkIfAdmin, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) diff --git a/pkg/ssh/cmd/delete.go b/pkg/ssh/cmd/delete.go index 09fe3de6b..b1f4f21b6 100644 --- a/pkg/ssh/cmd/delete.go +++ b/pkg/ssh/cmd/delete.go @@ -11,7 +11,7 @@ func deleteCommand() *cobra.Command { Aliases: []string{"del", "remove", "rm"}, Short: "Delete a repository", Args: cobra.ExactArgs(1), - PersistentPreRunE: checkIfReadableAndCollab, + PersistentPreRunE: checkIfAdmin, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) diff --git a/pkg/ssh/cmd/git.go b/pkg/ssh/cmd/git.go index caf1e1595..eb4b04171 100644 --- a/pkg/ssh/cmd/git.go +++ b/pkg/ssh/cmd/git.go @@ -1,6 +1,7 @@ package cmd import ( + "context" "errors" "path/filepath" "strings" @@ -209,7 +210,18 @@ func gitRunE(cmd *cobra.Command, args []string) error { if sess := sshutils.SessionFromContext(ctx); sess != nil { for _, env := range sess.Environ() { if strings.HasPrefix(env, "GIT_PROTOCOL=") { - envs = append(envs, env) + val := env[len("GIT_PROTOCOL="):] + // Sanitize: reject any control characters to prevent env var injection. + clean := true + for _, c := range val { + if c < 0x20 || c == 0x7f { + clean = false + break + } + } + if clean { + envs = append(envs, env) + } break } } @@ -262,8 +274,6 @@ func gitRunE(cmd *cobra.Command, args []string) error { return git.ErrSystemMalfunction } - receivePackCounter.WithLabelValues(name).Inc() - return nil case git.UploadPackService, git.UploadArchiveService: if accessLevel < access.ReadOnlyAccess { @@ -291,7 +301,19 @@ func gitRunE(cmd *cobra.Command, args []string) error { if errors.Is(err, git.ErrInvalidRepo) { return git.ErrInvalidRepo } else if err != nil { - logger.Error("failed to handle git service", "service", service, "err", err, "repo", name) + // When the client disconnects early (e.g. Flux Source controller + // closes the connection after receiving ref advertisements), the + // SSH session context is cancelled and exec.CommandContext kills + // the git subprocess with SIGKILL, producing "signal: killed". + // That is normal — log it at debug level, not error. + // Use context.Canceled specifically: context.DeadlineExceeded + // means a server-side timeout fired, which is still worth an + // ERROR so operators can tune their timeout settings. + if ctx.Err() == context.Canceled { + logger.Debug("git service ended on client disconnect", "service", service, "err", err, "repo", name) + } else { + logger.Error("failed to handle git service", "service", service, "err", err, "repo", name) + } return git.ErrSystemMalfunction } diff --git a/pkg/ssh/cmd/info.go b/pkg/ssh/cmd/info.go index bb932390a..a60c2c119 100644 --- a/pkg/ssh/cmd/info.go +++ b/pkg/ssh/cmd/info.go @@ -1,7 +1,6 @@ package cmd import ( - "github.com/charmbracelet/soft-serve/pkg/backend" "github.com/charmbracelet/soft-serve/pkg/sshutils" "github.com/spf13/cobra" ) @@ -14,9 +13,7 @@ func InfoCommand() *cobra.Command { Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() - be := backend.FromContext(ctx) - pk := sshutils.PublicKeyFromContext(ctx) - user, err := be.UserByPublicKey(ctx, pk) + user, err := currentUser(ctx) if err != nil { return err } diff --git a/pkg/ssh/cmd/list.go b/pkg/ssh/cmd/list.go index bcdfa99cd..655b0b9a8 100644 --- a/pkg/ssh/cmd/list.go +++ b/pkg/ssh/cmd/list.go @@ -3,7 +3,7 @@ package cmd import ( "github.com/charmbracelet/soft-serve/pkg/access" "github.com/charmbracelet/soft-serve/pkg/backend" - "github.com/charmbracelet/soft-serve/pkg/sshutils" + "github.com/charmbracelet/soft-serve/pkg/proto" "github.com/spf13/cobra" ) @@ -19,13 +19,13 @@ func listCommand() *cobra.Command { RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) - pk := sshutils.PublicKeyFromContext(ctx) + user := proto.UserFromContext(ctx) repos, err := be.Repositories(ctx) if err != nil { return err } for _, r := range repos { - if be.AccessLevelByPublicKey(ctx, r.Name(), pk) >= access.ReadOnlyAccess { + if be.AccessLevelForUser(ctx, r.Name(), user) >= access.ReadOnlyAccess { if !r.IsHidden() || all { cmd.Println(r.Name()) } diff --git a/pkg/ssh/cmd/pubkey.go b/pkg/ssh/cmd/pubkey.go index 697ab51c8..d3273ac6e 100644 --- a/pkg/ssh/cmd/pubkey.go +++ b/pkg/ssh/cmd/pubkey.go @@ -23,8 +23,7 @@ func PubkeyCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) - pk := sshutils.PublicKeyFromContext(ctx) - user, err := be.UserByPublicKey(ctx, pk) + user, err := currentUser(ctx) if err != nil { return err } @@ -45,8 +44,7 @@ func PubkeyCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) - pk := sshutils.PublicKeyFromContext(ctx) - user, err := be.UserByPublicKey(ctx, pk) + user, err := currentUser(ctx) if err != nil { return err } @@ -67,9 +65,7 @@ func PubkeyCommand() *cobra.Command { Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() - be := backend.FromContext(ctx) - pk := sshutils.PublicKeyFromContext(ctx) - user, err := be.UserByPublicKey(ctx, pk) + user, err := currentUser(ctx) if err != nil { return err } diff --git a/pkg/ssh/cmd/push_mirror.go b/pkg/ssh/cmd/push_mirror.go new file mode 100644 index 000000000..737650ee8 --- /dev/null +++ b/pkg/ssh/cmd/push_mirror.go @@ -0,0 +1,146 @@ +package cmd + +import ( + "errors" + "fmt" + "net/url" + "strconv" + "strings" + + "charm.land/lipgloss/v2/table" + "github.com/charmbracelet/soft-serve/pkg/backend" + "github.com/dustin/go-humanize" + "github.com/spf13/cobra" +) + +func pushMirrorCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "push-mirror", + Aliases: []string{"push-mirrors"}, + Short: "Manage repository push mirrors", + } + + cmd.AddCommand( + pushMirrorAddCommand(), + pushMirrorRemoveCommand(), + pushMirrorListCommand(), + ) + + return cmd +} + +func pushMirrorAddCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "add REPOSITORY NAME REMOTE_URL", + Short: "Add a push mirror to a repository", + Args: cobra.ExactArgs(3), + PersistentPreRunE: checkIfAdmin, + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + be := backend.FromContext(ctx) + + remoteURL := args[2] + if err := validateMirrorURL(remoteURL); err != nil { + return err + } + + // Warn when the remote uses plain HTTP — credentials and content + // will be sent in the clear over the network. + if u, err := url.Parse(remoteURL); err == nil && strings.EqualFold(u.Scheme, "http") { + fmt.Fprintln(cmd.ErrOrStderr(), "warning: push mirror uses plain HTTP — credentials and data are not encrypted in transit; consider using HTTPS or SSH") + } + + repo, err := be.Repository(ctx, args[0]) + if err != nil { + return err + } + return be.AddPushMirror(ctx, repo, args[1], remoteURL) + }, + } + return cmd +} + +func pushMirrorRemoveCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "remove REPOSITORY NAME", + Short: "Remove a push mirror from a repository", + Args: cobra.ExactArgs(2), + PersistentPreRunE: checkIfAdmin, + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + be := backend.FromContext(ctx) + repo, err := be.Repository(ctx, args[0]) + if err != nil { + return err + } + return be.RemovePushMirror(ctx, repo, args[1]) + }, + } + return cmd +} + +// validateMirrorURL ensures the remote URL uses an allowed protocol. +// Rejects file://, git://, and other protocols that could be used for SSRF. +func validateMirrorURL(rawURL string) error { + u, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("invalid mirror URL: %w", err) + } + switch strings.ToLower(u.Scheme) { + case "https", "http", "ssh", "git+ssh", "ssh+git": + // git+ssh:// and ssh+git:// are aliases for ssh://, supported by the + // push engine in pkg/backend/push_mirror.go. + return nil + case "": + // SCP-style SSH remotes (git@host:path) are allowed. + // Bare local paths like /etc/shadow or ./relative are not. + if strings.HasPrefix(u.Path, "/") || strings.HasPrefix(u.Path, ".") { + return errors.New("local file paths are not allowed as mirror remotes") + } + // For SCP-style, url.Parse puts the whole thing in Opaque or Path. + // Ensure there's a recognizable host:path separator (colon). + if !strings.Contains(rawURL, ":") { + return errors.New("invalid mirror remote URL: no host specified") + } + return nil + default: + return fmt.Errorf("mirror URL scheme %q is not allowed (use https, http, or ssh)", u.Scheme) + } +} + +func pushMirrorListCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "list REPOSITORY", + Short: "List push mirrors for a repository", + Args: cobra.ExactArgs(1), + PersistentPreRunE: checkIfReadable, + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + be := backend.FromContext(ctx) + repo, err := be.Repository(ctx, args[0]) + if err != nil { + return err + } + + mirrors, err := be.ListPushMirrors(ctx, repo) + if err != nil { + return err + } + + t := table.New().Headers("ID", "Name", "Remote URL", "Enabled", "Created At", "Updated At") + for _, m := range mirrors { + t = t.Row( + strconv.FormatInt(m.ID, 10), + m.Name, + m.RemoteURL, + strconv.FormatBool(m.Enabled), + humanize.Time(m.CreatedAt), + humanize.Time(m.UpdatedAt), + ) + } + cmd.Println(t) + return nil + }, + } + return cmd +} diff --git a/pkg/ssh/cmd/repo.go b/pkg/ssh/cmd/repo.go index fa3e72ff9..e7ce9e39c 100644 --- a/pkg/ssh/cmd/repo.go +++ b/pkg/ssh/cmd/repo.go @@ -1,9 +1,11 @@ package cmd import ( + "errors" "fmt" "strings" + "github.com/charmbracelet/soft-serve/git" "github.com/charmbracelet/soft-serve/pkg/backend" "github.com/charmbracelet/soft-serve/pkg/proto" "github.com/spf13/cobra" @@ -31,6 +33,7 @@ func RepoCommand() *cobra.Command { mirrorCommand(), privateCommand(), projectName(), + pushMirrorCommand(), renameCommand(), tagCommand(), treeCommand(), @@ -58,7 +61,8 @@ func RepoCommand() *cobra.Command { } head, err := r.HEAD() - if err != nil { + isEmpty := errors.Is(err, git.ErrReferenceNotExist) + if err != nil && !isEmpty { return err } @@ -84,7 +88,11 @@ func RepoCommand() *cobra.Command { if owner != nil { cmd.Println(strings.TrimSpace(fmt.Sprint("Owner: ", owner.Username()))) } - cmd.Println("Default Branch:", head.Name().Short()) + if isEmpty { + cmd.Println("Default Branch: (empty repository)") + } else { + cmd.Println("Default Branch:", head.Name().Short()) + } if len(branches) > 0 { cmd.Println("Branches:") for _, b := range branches { diff --git a/pkg/ssh/cmd/set_username.go b/pkg/ssh/cmd/set_username.go index 1fe93616b..6a46cb17a 100644 --- a/pkg/ssh/cmd/set_username.go +++ b/pkg/ssh/cmd/set_username.go @@ -2,7 +2,6 @@ package cmd import ( "github.com/charmbracelet/soft-serve/pkg/backend" - "github.com/charmbracelet/soft-serve/pkg/sshutils" "github.com/spf13/cobra" ) @@ -15,8 +14,7 @@ func SetUsernameCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) - pk := sshutils.PublicKeyFromContext(ctx) - user, err := be.UserByPublicKey(ctx, pk) + user, err := currentUser(ctx) if err != nil { return err } diff --git a/pkg/ssh/cmd/token.go b/pkg/ssh/cmd/token.go index 07b87ddf0..4b7e9580d 100644 --- a/pkg/ssh/cmd/token.go +++ b/pkg/ssh/cmd/token.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "strconv" "strings" "time" @@ -22,10 +23,15 @@ func TokenCommand() *cobra.Command { } var createExpiresIn string + var targetUsername string createCmd := &cobra.Command{ Use: "create NAME", Short: "Create a new access token", - Args: cobra.MinimumNArgs(1), + Long: `Create a new access token for the current user. + +Admins may supply --user to create a token on behalf of +another user, for example when provisioning CI/CD credentials.`, + Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) @@ -36,6 +42,18 @@ func TokenCommand() *cobra.Command { return proto.ErrUserNotFound } + targetUser := user + if targetUsername != "" { + if !user.IsAdmin() { + return fmt.Errorf("only admins can create tokens for other users") + } + tu, err := be.User(ctx, targetUsername) + if err != nil { + return fmt.Errorf("user %q not found: %w", targetUsername, err) + } + targetUser = tu + } + var expiresAt time.Time var expiresIn time.Duration if createExpiresIn != "" { @@ -48,7 +66,7 @@ func TokenCommand() *cobra.Command { expiresAt = time.Now().Add(d) } - token, err := be.CreateAccessToken(ctx, user, name, expiresAt) + token, err := be.CreateAccessToken(ctx, targetUser, name, expiresAt) if err != nil { return err } @@ -66,6 +84,7 @@ func TokenCommand() *cobra.Command { } createCmd.Flags().StringVar(&createExpiresIn, "expires-in", "", "Token expiration time (e.g. 1y, 3mo, 2w, 5d4h, 1h30m)") + createCmd.Flags().StringVarP(&targetUsername, "user", "u", "", "create token for this user (admin only)") listCmd := &cobra.Command{ Use: "list", diff --git a/pkg/ssh/cmd/user.go b/pkg/ssh/cmd/user.go index 21981f9cb..aea93ce3b 100644 --- a/pkg/ssh/cmd/user.go +++ b/pkg/ssh/cmd/user.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "sort" "strings" @@ -22,22 +23,45 @@ func UserCommand() *cobra.Command { var admin bool var key string userCreateCommand := &cobra.Command{ - Use: "create USERNAME", - Short: "Create a new user", - Args: cobra.ExactArgs(1), + Use: "create USERNAME", + Short: "Create a new user", + Long: `Create a new user. + +When passing a public key with -k, shell quoting is stripped by OpenSSH +before the command is transmitted, so an ed25519 key such as: + + ssh host user create alice -k 'ssh-ed25519 AAAA... user@host' + +arrives on the server as three separate tokens. Soft Serve re-joins them +automatically, so both of the following forms work: + + -k 'ssh-ed25519 AAAA... user@host' (quoted, local shell) + -k ssh-ed25519 AAAA... user@host (unquoted, same effect over SSH) +`, + Args: cobra.MinimumNArgs(1), PersistentPreRunE: checkIfAdmin, RunE: func(cmd *cobra.Command, args []string) error { var pubkeys []ssh.PublicKey ctx := cmd.Context() be := backend.FromContext(ctx) username := args[0] - if key != "" { - pk, _, err := sshutils.ParseAuthorizedKey(key) + + switch { + case cmd.Flags().Changed("key") && key == "": + // -k was supplied with an empty value. + return fmt.Errorf("flag --key requires a non-empty public key") + case cmd.Flags().Changed("key"): + // Re-join the -k value with any remaining positional args. + // This reconstructs a key split across tokens by SSH quoting + // stripping (e.g. 'ssh-ed25519 AAAA' → two tokens). + keyStr := strings.TrimSpace(strings.Join(append([]string{key}, args[1:]...), " ")) + pk, _, err := sshutils.ParseAuthorizedKey(keyStr) if err != nil { return err } - pubkeys = []ssh.PublicKey{pk} + case len(args) > 1: + return fmt.Errorf("unexpected arguments: %s", strings.Join(args[1:], " ")) } opts := proto.UserOptions{ diff --git a/pkg/ssh/cmd/user_test.go b/pkg/ssh/cmd/user_test.go new file mode 100644 index 000000000..a58f7d357 --- /dev/null +++ b/pkg/ssh/cmd/user_test.go @@ -0,0 +1,94 @@ +package cmd + +import ( + "strings" + "testing" + + "github.com/charmbracelet/soft-serve/pkg/sshutils" +) + +// reconstructKey mirrors the logic in userCreateCommand.RunE: +// join the -k flag value with any trailing positional args and parse. +func reconstructKey(flagValue string, extraArgs []string) (string, error) { + keyStr := strings.TrimSpace(strings.Join(append([]string{flagValue}, extraArgs...), " ")) + _, _, err := sshutils.ParseAuthorizedKey(keyStr) + return keyStr, err +} + +// TestUserCreateKeyReconstruction verifies the key-joining logic used when +// OpenSSH strips shell quoting and delivers the key as separate tokens. +// +// Note: the empty-key guard (cmd.Flags().Changed("key") && key == "") +// is exercised only via integration tests (testscript/testdata) because +// invoking RunE requires passing the checkIfAdmin PersistentPreRunE, +// which needs a full backend context. +func TestUserCreateKeyReconstruction(t *testing.T) { + // Generate a real ed25519 key in authorized_keys format for tests. + const testKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBzKEBMH+cKg8+8v7CJrbPBpbmMHbzSENKgmHhYRhM89 test@host" + keyType := "ssh-ed25519" + keyBody := "AAAAC3NzaC1lZDI1NTE5AAAAIBzKEBMH+cKg8+8v7CJrbPBpbmMHbzSENKgmHhYRhM89" + comment := "test@host" + + tests := []struct { + name string + flagValue string // value cobra captures for -k + extraArgs []string + wantErr bool + wantKey string // expected reconstructed key string + }{ + { + name: "full key in flag value (proper quoting preserved)", + flagValue: testKey, + extraArgs: nil, + wantKey: testKey, + }, + { + name: "key type in flag, body as extra arg (SSH quoting stripped)", + flagValue: keyType, + extraArgs: []string{keyBody}, + wantKey: keyType + " " + keyBody, + }, + { + name: "key type in flag, body and comment as extra args", + flagValue: keyType, + extraArgs: []string{keyBody, comment}, + wantKey: keyType + " " + keyBody + " " + comment, + }, + { + name: "key type only (missing body) returns parse error", + flagValue: keyType, + extraArgs: nil, + wantErr: true, + }, + { + name: "empty flag value returns parse error", + flagValue: "", + extraArgs: nil, + wantErr: true, + }, + { + name: "invalid base64 body returns parse error", + flagValue: keyType, + extraArgs: []string{"!!!not-base64!!!"}, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := reconstructKey(tc.flagValue, tc.extraArgs) + if tc.wantErr { + if err == nil { + t.Errorf("expected error, got key %q", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tc.wantKey { + t.Errorf("reconstructed key = %q; want %q", got, tc.wantKey) + } + }) + } +} diff --git a/pkg/ssh/cmd/webhooks.go b/pkg/ssh/cmd/webhooks.go index 6312b909b..ab8532f89 100644 --- a/pkg/ssh/cmd/webhooks.go +++ b/pkg/ssh/cmd/webhooks.go @@ -278,12 +278,16 @@ func webhookDeliveriesListCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) + repo, err := be.Repository(ctx, args[0]) + if err != nil { + return fmt.Errorf("repository not found: %w", err) + } id, err := strconv.ParseInt(args[1], 10, 64) if err != nil { return fmt.Errorf("invalid webhook ID: %w", err) } - dels, err := be.ListWebhookDeliveries(ctx, id) + dels, err := be.ListWebhookDeliveries(ctx, repo, id) if err != nil { return err } @@ -349,6 +353,11 @@ func webhookDeliveriesGetCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) + repo, err := be.Repository(ctx, args[0]) + if err != nil { + return err + } + id, err := strconv.ParseInt(args[1], 10, 64) if err != nil { return fmt.Errorf("invalid webhook ID: %w", err) @@ -359,7 +368,7 @@ func webhookDeliveriesGetCommand() *cobra.Command { return fmt.Errorf("invalid delivery ID: %w", err) } - del, err := be.WebhookDelivery(ctx, id, delID) + del, err := be.WebhookDelivery(ctx, repo, id, delID) if err != nil { return err } diff --git a/pkg/ssh/middleware.go b/pkg/ssh/middleware.go index 25a00b1a2..1b09e0174 100644 --- a/pkg/ssh/middleware.go +++ b/pkg/ssh/middleware.go @@ -3,6 +3,7 @@ package ssh import ( "fmt" "strconv" + "strings" "time" "charm.land/log/v2" @@ -55,17 +56,37 @@ func AuthenticationMiddleware(sh ssh.Handler) ssh.Handler { return } + // Check if this session was authenticated via access token. + // Token-authenticated sessions carry the user ID via a package-private + // context key (tokenAuthUserIDKey) set during keyboard-interactive auth. + // We intentionally do NOT read this from perms.Extensions — certificate + // extensions from gossh are merged into the same map, so a string key + // can be injected by a client presenting a crafted certificate. + _, isTokenAuth := ctx.Value(tokenAuthUserIDKey{}).(int64) + ac := be.AllowKeyless(ctx) - publicKeyCounter.WithLabelValues(strconv.FormatBool(ac || pk != nil)).Inc() - if !ac && pk == nil { + publicKeyCounter.WithLabelValues(strconv.FormatBool(ac || pk != nil || isTokenAuth)).Inc() + if !ac && pk == nil && !isTokenAuth { wish.Fatalln(s, ErrPermissionDenied) return } - // Set the auth'd user, or anon, in the context + // Set the auth'd user, or anon, in the context. var user proto.User if pk != nil { - user, _ = be.UserByPublicKey(ctx, pk) + if u, err := be.UserByPublicKey(ctx, pk); err != nil { + log.FromContext(ctx).Debug("public key lookup failed", "err", err) + } else { + user = u + } + } else if tokenID, ok := ctx.Value(tokenAuthUserIDKey{}).(int64); ok { + if u, err := be.UserByID(ctx, tokenID); err != nil { + log.FromContext(ctx).Warn("failed to resolve token-auth user", "id", tokenID, "err", err) + wish.Fatalln(s, ErrPermissionDenied) + return + } else { + user = u + } } ctx.SetValue(proto.ContextKeyUser, user) @@ -195,9 +216,15 @@ func LoggingMiddleware(sh ssh.Handler) ssh.Handler { } if config.IsVerbose() { + safeEnvs := make([]string, 0, len(s.Environ())) + for _, e := range s.Environ() { + if strings.HasPrefix(e, "GIT_") || e == "TERM" || e == "LANG" || strings.HasPrefix(e, "LC_") { + safeEnvs = append(safeEnvs, e) + } + } logArgs = append(logArgs, "key", hpk, - "envs", s.Environ(), + "envs", safeEnvs, ) } diff --git a/pkg/ssh/middleware_test.go b/pkg/ssh/middleware_test.go index 149fd8d8b..ff235be2e 100644 --- a/pkg/ssh/middleware_test.go +++ b/pkg/ssh/middleware_test.go @@ -139,12 +139,16 @@ func TestAuthenticationBypass(t *testing.T) { is.Equal(authenticatedUser.Username(), "testattacker") is.True(!authenticatedUser.IsAdmin()) - // If the vulnerability exists, the context would still have admin user - contextUser := proto.UserFromContext(mockCtx) - if contextUser != nil && contextUser.Username() == "testadmin" { - t.Logf("WARNING: Context still contains admin user! This indicates the vulnerability exists.") - t.Logf("The authenticated key is attacker's, but context has admin user.") - } + // TODO: add integration test that exercises AuthenticationMiddleware directly + // In this simulation the mock context still holds the admin user because + // AuthenticationMiddleware has NOT been invoked — only the raw backend lookup + // above was exercised. In production, AuthenticationMiddleware re-looks up the + // user from the authenticated pubkey fingerprint stored in SSH extensions + // (pubkey-fp), overwriting any stale context value set by an earlier + // PublicKeyHandler call. The assertions above (lines 135-140) confirm that the + // backend correctly resolves the attacker's key to "testattacker", so the + // middleware would store the right user. The pre-polluted context value is + // therefore harmless in the real code path. }) } diff --git a/pkg/ssh/session.go b/pkg/ssh/session.go index 295542d5e..3cb8dcca3 100644 --- a/pkg/ssh/session.go +++ b/pkg/ssh/session.go @@ -47,7 +47,7 @@ func SessionHandler(s ssh.Session) *tea.Program { initialRepo = cmd[0] } - auth := be.AccessLevelByPublicKey(ctx, initialRepo, s.PublicKey()) + auth := be.AccessLevelForUser(ctx, initialRepo, proto.UserFromContext(ctx)) if auth < access.ReadOnlyAccess { wish.Fatalln(s, proto.ErrUnauthorized) return nil diff --git a/pkg/ssh/ssh.go b/pkg/ssh/ssh.go index 6fc9fcdf8..3129260cb 100644 --- a/pkg/ssh/ssh.go +++ b/pkg/ssh/ssh.go @@ -5,6 +5,7 @@ import ( "fmt" "net" "os" + "path/filepath" "strconv" "time" @@ -16,11 +17,13 @@ import ( "github.com/charmbracelet/soft-serve/pkg/backend" "github.com/charmbracelet/soft-serve/pkg/config" "github.com/charmbracelet/soft-serve/pkg/db" + "github.com/charmbracelet/soft-serve/pkg/ratelimit" "github.com/charmbracelet/soft-serve/pkg/store" "github.com/charmbracelet/ssh" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" gossh "golang.org/x/crypto/ssh" + "golang.org/x/time/rate" ) var ( @@ -39,13 +42,20 @@ var ( }, []string{"allowed"}) ) +// tokenAuthUserIDKey is a package-private context key used to carry the +// token-authenticated user ID from KeyboardInteractiveHandler to +// AuthenticationMiddleware. Using a private type (not a string) prevents +// injection via SSH certificate extensions, which use string-keyed maps. +type tokenAuthUserIDKey struct{} + // SSHServer is a SSH server that implements the git protocol. type SSHServer struct { //nolint: revive - srv *ssh.Server - cfg *config.Config - be *backend.Backend - ctx context.Context - logger *log.Logger + srv *ssh.Server + cfg *config.Config + be *backend.Backend + ctx context.Context + logger *log.Logger + limiter *ratelimit.IPLimiter } // NewSSHServer returns a new SSHServer. @@ -63,6 +73,7 @@ func NewSSHServer(ctx context.Context) (*SSHServer, error) { be: be, logger: logger, } + s.limiter = ratelimit.New(rate.Limit(10), 20, 5*time.Minute) mw := []wish.Middleware{ rm.MiddlewareWithLogger( @@ -84,9 +95,22 @@ func NewSSHServer(ctx context.Context) (*SSHServer, error) { ), } + // Ensure the directory for the host key file exists. + if dir := filepath.Dir(cfg.SSH.KeyPath); dir != "." { + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, fmt.Errorf("create ssh key dir: %w", err) + } + } + opts := []ssh.Option{ ssh.PublicKeyAuth(s.PublicKeyHandler), - ssh.KeyboardInteractiveAuth(s.KeyboardInteractiveHandler), + ssh.KeyboardInteractiveAuth(func(ctx ssh.Context, challenge gossh.KeyboardInteractiveChallenge) bool { + if !s.limiter.Allow(ctx.RemoteAddr().String()) { + s.logger.Warn("SSH keyboard-interactive rate limited", "remote", ctx.RemoteAddr()) + return false + } + return s.KeyboardInteractiveHandler(ctx, challenge) + }), wish.WithAddress(cfg.SSH.ListenAddr), wish.WithHostKeyPath(cfg.SSH.KeyPath), wish.WithMiddleware(mw...), @@ -100,14 +124,23 @@ func NewSSHServer(ctx context.Context) (*SSHServer, error) { return nil, err } - if config.IsDebug() { - s.srv.ServerConfigCallback = func(_ ssh.Context) *gossh.ServerConfig { - return &gossh.ServerConfig{ - AuthLogCallback: func(conn gossh.ConnMetadata, method string, err error) { - logger.Debug("authentication", "user", conn.User(), "method", method, "err", err) - }, + s.srv.ServerConfigCallback = func(_ ssh.Context) *gossh.ServerConfig { + sc := &gossh.ServerConfig{} + if len(cfg.SSH.KeyExchanges) > 0 { + sc.KeyExchanges = cfg.SSH.KeyExchanges + } + if len(cfg.SSH.Ciphers) > 0 { + sc.Ciphers = cfg.SSH.Ciphers + } + if len(cfg.SSH.MACs) > 0 { + sc.MACs = cfg.SSH.MACs + } + if config.IsDebug() { + sc.AuthLogCallback = func(conn gossh.ConnMetadata, method string, err error) { + logger.Debug("authentication", "user", conn.User(), "method", method, "err", err) } } + return sc } if cfg.SSH.MaxTimeout > 0 { @@ -157,6 +190,7 @@ func initializePermissions(ctx ssh.Context) { if perms.Extensions == nil { perms.Extensions = make(map[string]string) } + ctx.SetValue(ssh.ContextKeyPermissions, perms) } // PublicKeyHandler handles public key authentication. @@ -172,28 +206,61 @@ func (s *SSHServer) PublicKeyHandler(ctx ssh.Context, pk ssh.PublicKey) (allowed initializePermissions(ctx) perms := ctx.Permissions() - // Set the public key fingerprint to be used for authentication. - perms.Extensions["pubkey-fp"] = gossh.FingerprintSHA256(pk) - ctx.SetValue(ssh.ContextKeyPermissions, perms) + // Only record the first offered key — the SSH client will use this + // key for the authenticated signature step. Overwriting on each probe + // would store the LAST key offered, which may differ from the one the + // client ultimately signs with, causing a fingerprint mismatch in + // AuthenticationMiddleware. + if perms.Extensions["pubkey-fp"] == "" { + perms.Extensions["pubkey-fp"] = gossh.FingerprintSHA256(pk) + ctx.SetValue(ssh.ContextKeyPermissions, perms) + } return } // KeyboardInteractiveHandler handles keyboard interactive authentication. -// This is used after all public key authentication has failed. -func (s *SSHServer) KeyboardInteractiveHandler(ctx ssh.Context, _ gossh.KeyboardInteractiveChallenge) bool { - ac := s.be.AllowKeyless(ctx) - keyboardInteractiveCounter.WithLabelValues(strconv.FormatBool(ac)).Inc() - - // If we're allowing keyless access, reset the public key fingerprint +// It prompts for an access token and validates it. If no valid token is +// provided, it falls back to AllowKeyless behavior. +func (s *SSHServer) KeyboardInteractiveHandler(ctx ssh.Context, challenge gossh.KeyboardInteractiveChallenge) bool { initializePermissions(ctx) perms := ctx.Permissions() + // Prompt the user for an access token. + answers, err := challenge("", "", []string{"Access Token: "}, []bool{false}) + if err != nil { + s.logger.Debug("keyboard-interactive challenge failed", "err", err) + } else if len(answers) > 0 && answers[0] != "" { + token := answers[0] + user, tokenErr := s.be.UserByAccessToken(ctx, token) + if tokenErr == nil && user != nil { + // Valid token: store the user ID via a package-private context key. + // We intentionally do NOT use perms.Extensions here — certificate + // extensions from gossh are merged into the same map, so a string + // key can be injected by a client presenting a crafted certificate. + // + // Clear pubkey-fp so AuthenticationMiddleware's fingerprint guard + // does not reject this keyless token-auth session. + perms.Extensions["pubkey-fp"] = "" + ctx.SetValue(ssh.ContextKeyPermissions, perms) + ctx.SetValue(tokenAuthUserIDKey{}, user.ID()) + keyboardInteractiveCounter.WithLabelValues("true").Inc() + s.logger.Info("keyboard-interactive token auth succeeded", "username", user.Username()) + return true + } + if tokenErr != nil { + s.logger.Warn("keyboard-interactive token auth failed", "err", tokenErr) + } else { + s.logger.Warn("keyboard-interactive token auth failed", "err", "user not found") + } + } + + // No valid token: fall back to AllowKeyless behavior. + ac := s.be.AllowKeyless(ctx) + keyboardInteractiveCounter.WithLabelValues(strconv.FormatBool(ac)).Inc() + if ac { - // XXX: reset the public-key fingerprint. This is used to validate the - // public key being used to authenticate. perms.Extensions["pubkey-fp"] = "" - ctx.SetValue(ssh.ContextKeyPermissions, perms) } return ac } diff --git a/pkg/ssh/ui.go b/pkg/ssh/ui.go index 2e7d7b615..2d4b8c0c6 100644 --- a/pkg/ssh/ui.go +++ b/pkg/ssh/ui.go @@ -140,6 +140,7 @@ func (ui *UI) Init() tea.Cmd { repo.NewLog(ui.common), repo.NewRefs(ui.common, git.RefsHeads), repo.NewRefs(ui.common, git.RefsTags), + repo.NewDiff(ui.common), ) ui.SetSize(ui.common.Width, ui.common.Height) cmds := make([]tea.Cmd, 0) @@ -259,7 +260,12 @@ func (ui *UI) Update(msg tea.Msg) (tea.Model, tea.Cmd) { func (ui *UI) View() tea.View { var v tea.View v.AltScreen = true - v.MouseMode = tea.MouseModeCellMotion + // Respect the allow_mouse_events config: when false, disable TUI mouse + // capture so the terminal's native text-selection works instead. + cfg := ui.common.Config() + if cfg == nil || cfg.SSH.AllowMouseEvents { + v.MouseMode = tea.MouseModeCellMotion + } var view string wm, hm := ui.getMargins() diff --git a/pkg/ssrf/ssrf.go b/pkg/ssrf/ssrf.go index 475c1cd72..01cdbed3f 100644 --- a/pkg/ssrf/ssrf.go +++ b/pkg/ssrf/ssrf.go @@ -39,14 +39,28 @@ func NewSecureClient() *http.Client { ip := net.ParseIP(host) if ip == nil { - ips, err := net.LookupIP(host) //nolint + // Use the context-aware resolver so the lookup respects + // cancellation and deadlines from the caller. + addrs, err := net.DefaultResolver.LookupIPAddr(ctx, host) if err != nil { return nil, fmt.Errorf("DNS resolution failed for host %s: %v", host, err) } - if len(ips) == 0 { + if len(addrs) == 0 { return nil, fmt.Errorf("no IP addresses found for host: %s", host) } - ip = ips[0] // Use the first resolved IP address + // Reject if ANY resolved IP is private/internal. + // Select the first public IP for dialing so that DNS + // round-robin cannot swap in a private address on retry. + var selectedIP net.IP + for _, addr := range addrs { + if isPrivateOrInternal(addr.IP) { + return nil, fmt.Errorf("%w", ErrPrivateIP) + } + if selectedIP == nil { + selectedIP = addr.IP + } + } + ip = selectedIP } if isPrivateOrInternal(ip) { return nil, fmt.Errorf("%w", ErrPrivateIP) @@ -60,6 +74,11 @@ func NewSecureClient() *http.Client { // Without this, the dialer resolves the hostname again // independently, and the second resolution could return // a different (private) IP. + // Note: creating a new dialer per call is wasteful but + // ensures each connection has a fresh resolver state + // (safe against cache poisoning). For high-throughput + // webhook delivery, consider reusing a single dialer outside + // this closure if performance becomes a bottleneck. return dialer.DialContext(ctx, network, net.JoinHostPort(ip.String(), port)) }, MaxIdleConns: 100, @@ -67,6 +86,10 @@ func NewSecureClient() *http.Client { TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, }, + // Refuse all HTTP redirects. For webhooks this prevents SSRF via a + // redirect to an internal host. For LFS this is safe because the LFS + // batch API returns explicit download/upload href values; the LFS client + // calls those URLs directly and does not rely on server-issued redirects. CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, @@ -81,6 +104,8 @@ func isPrivateOrInternal(ip net.IP) bool { ip = ip4 } + // ip.IsPrivate() covers IPv6 ULA (fc00::/7) and IPv4 private ranges; + // no separate fc00::/7 check is needed here. if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsPrivate() || ip.IsUnspecified() || ip.IsMulticast() { return true @@ -126,8 +151,9 @@ func isPrivateOrInternal(ip net.IP) bool { // ValidateURL validates that a URL is safe to make requests to. // It checks that the scheme is http/https, the hostname is not localhost, -// and all resolved IPs are public. -func ValidateURL(rawURL string) error { +// and all resolved IPs are public. The provided context is used for the DNS +// lookup; a 5-second sub-deadline is applied if the context has no deadline. +func ValidateURL(ctx context.Context, rawURL string) error { if rawURL == "" { return ErrInvalidURL } @@ -157,7 +183,9 @@ func ValidateURL(rawURL string) error { return nil } - ips, err := net.DefaultResolver.LookupIPAddr(context.Background(), hostname) + resolveCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + ips, err := net.DefaultResolver.LookupIPAddr(resolveCtx, hostname) if err != nil { return fmt.Errorf("%w: cannot resolve hostname: %v", ErrInvalidURL, err) } @@ -180,6 +208,46 @@ func ValidateIPBeforeDial(ip net.IP) error { return nil } +// ValidateHost resolves host and checks that none of the resolved IPs are +// private or internal. Use this for non-HTTP schemes (e.g. ssh://) where +// ValidateURL cannot be used. The provided context is used for the DNS lookup. +// +// A 5-second sub-deadline is applied for the DNS lookup regardless of any +// deadline already present on ctx. If ctx has a tighter deadline, that takes +// precedence. If ctx has a longer (or no) deadline, the 5-second guard is the +// effective timeout for the DNS resolution step. +func ValidateHost(ctx context.Context, host string) error { + if host == "" { + return fmt.Errorf("%w: missing hostname", ErrInvalidURL) + } + + if isLocalhost(host) { + return ErrPrivateIP + } + + if ip := net.ParseIP(host); ip != nil { + if isPrivateOrInternal(ip) { + return ErrPrivateIP + } + return nil + } + + resolveCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + ips, err := net.DefaultResolver.LookupIPAddr(resolveCtx, host) + if err != nil { + return fmt.Errorf("%w: cannot resolve hostname: %v", ErrInvalidURL, err) + } + + if slices.ContainsFunc(ips, func(addr net.IPAddr) bool { + return isPrivateOrInternal(addr.IP) + }) { + return ErrPrivateIP + } + + return nil +} + // isLocalhost checks if the hostname is localhost or similar. func isLocalhost(hostname string) bool { hostname = strings.ToLower(hostname) diff --git a/pkg/ssrf/ssrf_test.go b/pkg/ssrf/ssrf_test.go index 1b1419469..486a4daaf 100644 --- a/pkg/ssrf/ssrf_test.go +++ b/pkg/ssrf/ssrf_test.go @@ -191,7 +191,7 @@ func TestValidateURL(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := ValidateURL(tt.url) + err := ValidateURL(context.Background(), tt.url) if (err != nil) != tt.wantErr { t.Errorf("ValidateURL(%q) error = %v, wantErr %v", tt.url, err, tt.wantErr) return diff --git a/pkg/stats/stats.go b/pkg/stats/stats.go index 0ee72963e..d6cd61edf 100644 --- a/pkg/stats/stats.go +++ b/pkg/stats/stats.go @@ -2,6 +2,7 @@ package stats import ( "context" + "net" "net/http" "time" @@ -35,6 +36,13 @@ func NewStatsServer(ctx context.Context) (*StatsServer, error) { }, nil } +// Serve accepts connections on l and serves HTTP requests. +// The stats endpoint intentionally does not support TLS; it is expected +// to be exposed only on a loopback or internal network interface. +func (s *StatsServer) Serve(l net.Listener) error { + return s.server.Serve(l) +} + // ListenAndServe starts the StatsServer. func (s *StatsServer) ListenAndServe() error { return s.server.ListenAndServe() diff --git a/pkg/storage/local.go b/pkg/storage/local.go index bf2fac9a5..96cb75d85 100644 --- a/pkg/storage/local.go +++ b/pkg/storage/local.go @@ -2,6 +2,7 @@ package storage import ( "errors" + "fmt" "io" "io/fs" "os" @@ -24,30 +25,42 @@ func NewLocalStorage(root string) *LocalStorage { // Delete implements Storage. func (l *LocalStorage) Delete(name string) error { - name = l.fixPath(name) - return os.Remove(name) + p, err := l.fixPath(name) + if err != nil { + return err + } + return os.Remove(p) } // Open implements Storage. func (l *LocalStorage) Open(name string) (Object, error) { - name = l.fixPath(name) - return os.Open(name) + p, err := l.fixPath(name) + if err != nil { + return nil, err + } + return os.Open(p) } // Stat implements Storage. func (l *LocalStorage) Stat(name string) (fs.FileInfo, error) { - name = l.fixPath(name) - return os.Stat(name) + p, err := l.fixPath(name) + if err != nil { + return nil, err + } + return os.Stat(p) } // Put implements Storage. func (l *LocalStorage) Put(name string, r io.Reader) (int64, error) { - name = l.fixPath(name) - if err := os.MkdirAll(filepath.Dir(name), os.ModePerm); err != nil { + p, err := l.fixPath(name) + if err != nil { + return 0, err + } + if err := os.MkdirAll(filepath.Dir(p), os.ModePerm); err != nil { return 0, err } - f, err := os.Create(name) + f, err := os.Create(p) if err != nil { return 0, err } @@ -57,8 +70,11 @@ func (l *LocalStorage) Put(name string, r io.Reader) (int64, error) { // Exists implements Storage. func (l *LocalStorage) Exists(name string) (bool, error) { - name = l.fixPath(name) - _, err := os.Stat(name) + p, err := l.fixPath(name) + if err != nil { + return false, err + } + _, err = os.Stat(p) if err == nil { return true, nil } @@ -70,21 +86,43 @@ func (l *LocalStorage) Exists(name string) (bool, error) { // Rename implements Storage. func (l *LocalStorage) Rename(oldName, newName string) error { - oldName = l.fixPath(oldName) - newName = l.fixPath(newName) - if err := os.MkdirAll(filepath.Dir(newName), os.ModePerm); err != nil { + oldPath, err := l.fixPath(oldName) + if err != nil { + return err + } + newPath, err := l.fixPath(newName) + if err != nil { return err } + if err := os.MkdirAll(filepath.Dir(newPath), os.ModePerm); err != nil { + return err + } + + // If destination already exists the object was uploaded concurrently. + // Remove the temp file and return success — the stored copy is authoritative. + if _, err := os.Stat(newPath); err == nil { + _ = os.Remove(oldPath) + return nil + } - return os.Rename(oldName, newName) + return os.Rename(oldPath, newPath) } -// Replace all slashes with the OS-specific separator -func (l LocalStorage) fixPath(path string) string { +// fixPath resolves the storage-relative path and verifies it stays within the root. +// Replace all slashes with the OS-specific separator. +func (l LocalStorage) fixPath(path string) (string, error) { + if l.root == "" { + return "", fmt.Errorf("storage: empty root path") + } path = strings.ReplaceAll(path, "/", string(os.PathSeparator)) - if !filepath.IsAbs(path) { - return filepath.Join(l.root, path) + p := filepath.Join(l.root, path) + // Ensure the resolved path is within the storage root. + // Note: filepath.Join (used to build p) already resolves ".." sequences, + // so path traversal via ".." is not possible. The HasPrefix check guards + // against any residual escapes. + root := l.root + string(filepath.Separator) + if !strings.HasPrefix(p, root) && p != l.root { + return "", fmt.Errorf("storage: path %q escapes root", path) } - - return path + return p, nil } diff --git a/pkg/store/database/database.go b/pkg/store/database/database.go index 9e56ab646..f1a1c5dbb 100644 --- a/pkg/store/database/database.go +++ b/pkg/store/database/database.go @@ -22,6 +22,7 @@ type datastore struct { *lfsStore *accessTokenStore *webhookStore + *pushMirrorStore } // New returns a new store.Store database. @@ -41,6 +42,8 @@ func New(ctx context.Context, db *db.DB) store.Store { collabStore: &collabStore{}, lfsStore: &lfsStore{}, accessTokenStore: &accessTokenStore{}, + webhookStore: &webhookStore{}, + pushMirrorStore: &pushMirrorStore{}, } return s diff --git a/pkg/store/database/lfs.go b/pkg/store/database/lfs.go index 8179d1803..30632370c 100644 --- a/pkg/store/database/lfs.go +++ b/pkg/store/database/lfs.go @@ -2,26 +2,37 @@ package database import ( "context" + "fmt" + "path" "strings" "github.com/charmbracelet/soft-serve/pkg/db" "github.com/charmbracelet/soft-serve/pkg/db/models" "github.com/charmbracelet/soft-serve/pkg/store" + "github.com/jmoiron/sqlx" ) type lfsStore struct{} var _ store.LFSStore = (*lfsStore)(nil) -func sanitizePath(path string) string { - path = strings.TrimSpace(path) - path = strings.TrimPrefix(path, "/") - return path +func sanitizePath(p string) (string, error) { + p = strings.TrimSpace(p) + p = strings.TrimPrefix(p, "/") + p = path.Clean(p) + if strings.HasPrefix(p, "..") { + return "", fmt.Errorf("invalid lock path: %q", p) + } + return p, nil } // CreateLFSLockForUser implements store.LFSStore. func (*lfsStore) CreateLFSLockForUser(ctx context.Context, tx db.Handler, repoID int64, userID int64, path string, refname string) error { - path = sanitizePath(path) + var err error + path, err = sanitizePath(path) + if err != nil { + return err + } query := tx.Rebind(`INSERT INTO lfs_locks (repo_id, user_id, path, refname, updated_at) VALUES ( ?, @@ -31,7 +42,7 @@ func (*lfsStore) CreateLFSLockForUser(ctx context.Context, tx db.Handler, repoID CURRENT_TIMESTAMP ); `) - _, err := tx.ExecContext(ctx, query, repoID, userID, path, refname) + _, err = tx.ExecContext(ctx, query, repoID, userID, path, refname) return db.WrapError(err) } @@ -87,27 +98,35 @@ func (*lfsStore) GetLFSLocksForUser(ctx context.Context, tx db.Handler, repoID i // GetLFSLocksForPath implements store.LFSStore. func (*lfsStore) GetLFSLockForPath(ctx context.Context, tx db.Handler, repoID int64, path string) (models.LFSLock, error) { - path = sanitizePath(path) + var err error + path, err = sanitizePath(path) + if err != nil { + return models.LFSLock{}, err + } var lock models.LFSLock query := tx.Rebind(` SELECT * FROM lfs_locks WHERE repo_id = ? AND path = ?; `) - err := tx.GetContext(ctx, &lock, query, repoID, path) + err = tx.GetContext(ctx, &lock, query, repoID, path) return lock, db.WrapError(err) } // GetLFSLockForUserPath implements store.LFSStore. func (*lfsStore) GetLFSLockForUserPath(ctx context.Context, tx db.Handler, repoID int64, userID int64, path string) (models.LFSLock, error) { - path = sanitizePath(path) + var err error + path, err = sanitizePath(path) + if err != nil { + return models.LFSLock{}, err + } var lock models.LFSLock query := tx.Rebind(` SELECT * FROM lfs_locks WHERE repo_id = ? AND user_id = ? AND path = ?; `) - err := tx.GetContext(ctx, &lock, query, repoID, userID, path) + err = tx.GetContext(ctx, &lock, query, repoID, userID, path) return lock, db.WrapError(err) } @@ -177,6 +196,23 @@ func (*lfsStore) GetLFSObjectByOid(ctx context.Context, tx db.Handler, repoID in return obj, db.WrapError(err) } +// GetLFSObjectsByOids implements store.LFSStore. +// It returns the LFS objects for the given OIDs in a single IN query, +// eliminating the N+1 pattern in the batch download handler. +func (*lfsStore) GetLFSObjectsByOids(ctx context.Context, tx db.Handler, repoID int64, oids []string) ([]models.LFSObject, error) { + if len(oids) == 0 { + return nil, nil + } + query, args2, err := sqlx.In(`SELECT * FROM lfs_objects WHERE repo_id = ? AND oid IN (?);`, repoID, oids) + if err != nil { + return nil, err + } + query = tx.Rebind(query) + var objs []models.LFSObject + err = tx.SelectContext(ctx, &objs, query, args2...) + return objs, db.WrapError(err) +} + // GetLFSObjects implements store.LFSStore. func (*lfsStore) GetLFSObjects(ctx context.Context, tx db.Handler, repoID int64) ([]models.LFSObject, error) { var objs []models.LFSObject diff --git a/pkg/store/database/push_mirror.go b/pkg/store/database/push_mirror.go new file mode 100644 index 000000000..66ab70400 --- /dev/null +++ b/pkg/store/database/push_mirror.go @@ -0,0 +1,42 @@ +package database + +import ( + "context" + + "github.com/charmbracelet/soft-serve/pkg/db" + "github.com/charmbracelet/soft-serve/pkg/db/models" + "github.com/charmbracelet/soft-serve/pkg/store" +) + +type pushMirrorStore struct{} + +var _ store.PushMirrorStore = (*pushMirrorStore)(nil) + +// CreatePushMirror creates a push mirror for a repository. +func (*pushMirrorStore) CreatePushMirror(ctx context.Context, h db.Handler, repoID int64, name, remoteURL string) error { + query := h.Rebind(`INSERT INTO push_mirrors (repo_id, name, remote_url) VALUES (?, ?, ?);`) + _, err := h.ExecContext(ctx, query, repoID, name, remoteURL) + return db.WrapError(err) +} + +// DeletePushMirror deletes a push mirror by repo and name. +func (*pushMirrorStore) DeletePushMirror(ctx context.Context, h db.Handler, repoID int64, name string) error { + query := h.Rebind(`DELETE FROM push_mirrors WHERE repo_id = ? AND name = ?;`) + _, err := h.ExecContext(ctx, query, repoID, name) + return db.WrapError(err) +} + +// GetPushMirrorsByRepoID returns all push mirrors for a repository. +func (*pushMirrorStore) GetPushMirrorsByRepoID(ctx context.Context, h db.Handler, repoID int64) ([]models.PushMirror, error) { + var mirrors []models.PushMirror + query := h.Rebind(`SELECT id, repo_id, name, remote_url, enabled, created_at, updated_at FROM push_mirrors WHERE repo_id = ? ORDER BY name;`) + err := h.SelectContext(ctx, &mirrors, query, repoID) + return mirrors, db.WrapError(err) +} + +// SetPushMirrorEnabled enables or disables a push mirror. +func (*pushMirrorStore) SetPushMirrorEnabled(ctx context.Context, h db.Handler, repoID int64, name string, enabled bool) error { + query := h.Rebind(`UPDATE push_mirrors SET enabled = ?, updated_at = CURRENT_TIMESTAMP WHERE repo_id = ? AND name = ?;`) + _, err := h.ExecContext(ctx, query, enabled, repoID, name) + return db.WrapError(err) +} diff --git a/pkg/store/database/webhooks.go b/pkg/store/database/webhooks.go index 1bc1f206c..b1c3ee176 100644 --- a/pkg/store/database/webhooks.go +++ b/pkg/store/database/webhooks.go @@ -2,6 +2,7 @@ package database import ( "context" + "strings" "github.com/charmbracelet/soft-serve/pkg/db" "github.com/charmbracelet/soft-serve/pkg/db/models" @@ -41,15 +42,24 @@ func (*webhookStore) CreateWebhookDelivery(ctx context.Context, h db.Handler, id // CreateWebhookEvents implements store.WebhookStore. func (*webhookStore) CreateWebhookEvents(ctx context.Context, h db.Handler, webhookID int64, events []int) error { - query := h.Rebind(`INSERT INTO webhook_events (webhook_id, event) - VALUES (?, ?);`) - for _, event := range events { - _, err := h.ExecContext(ctx, query, webhookID, event) - if err != nil { - return err + if len(events) == 0 { + return nil + } + // Bulk INSERT to avoid N round-trips for webhooks with many events. + // Use strings.Builder to build the placeholder list without intermediate + // string allocations from strings.Join over a []string. + var pb strings.Builder + args := make([]interface{}, 0, len(events)*2) + for i, event := range events { + if i > 0 { + pb.WriteString(", ") } + pb.WriteString("(?, ?)") + args = append(args, webhookID, event) } - return nil + query := h.Rebind(`INSERT INTO webhook_events (webhook_id, event) VALUES ` + pb.String()) + _, err := h.ExecContext(ctx, query, args...) + return err } // DeleteWebhookByID implements store.WebhookStore. @@ -73,7 +83,7 @@ func (*webhookStore) DeleteWebhookDeliveryByID(ctx context.Context, h db.Handler return err } -// DeleteWebhookEventsByWebhookID implements store.WebhookStore. +// DeleteWebhookEventsByID implements store.WebhookStore. func (*webhookStore) DeleteWebhookEventsByID(ctx context.Context, h db.Handler, ids []int64) error { query, args, err := sqlx.In(`DELETE FROM webhook_events WHERE id IN (?);`, ids) if err != nil { @@ -94,8 +104,10 @@ func (*webhookStore) GetWebhookByID(ctx context.Context, h db.Handler, repoID in } // GetWebhookDeliveriesByWebhookID implements store.WebhookStore. +// Returns the most recent 100 deliveries to prevent unbounded memory use. +// Callers that need older deliveries should use a paginated query. func (*webhookStore) GetWebhookDeliveriesByWebhookID(ctx context.Context, h db.Handler, webhookID int64) ([]models.WebhookDelivery, error) { - query := h.Rebind(`SELECT * FROM webhook_deliveries WHERE webhook_id = ?;`) + query := h.Rebind(`SELECT * FROM webhook_deliveries WHERE webhook_id = ? ORDER BY created_at DESC LIMIT 100;`) var whds []models.WebhookDelivery err := h.SelectContext(ctx, &whds, query, webhookID) return whds, err @@ -125,17 +137,52 @@ func (*webhookStore) GetWebhookEventsByWebhookID(ctx context.Context, h db.Handl return whes, err } +// sqliteMaxPlaceholders is the maximum number of placeholders SQLite allows in +// a single query (SQLITE_MAX_VARIABLE_NUMBER, default 999). We batch queries +// larger than this to stay within the limit on both SQLite and PostgreSQL. +const sqliteMaxPlaceholders = 999 + +// GetWebhookEventsByWebhookIDs implements store.WebhookStore. +func (*webhookStore) GetWebhookEventsByWebhookIDs(ctx context.Context, h db.Handler, webhookIDs []int64) ([]models.WebhookEvent, error) { + if len(webhookIDs) == 0 { + return nil, nil + } + var all []models.WebhookEvent + for i := 0; i < len(webhookIDs); i += sqliteMaxPlaceholders { + end := i + sqliteMaxPlaceholders + if end > len(webhookIDs) { + end = len(webhookIDs) + } + batch := webhookIDs[i:end] + query, args, err := sqlx.In(`SELECT * FROM webhook_events WHERE webhook_id IN (?);`, batch) + if err != nil { + return nil, err + } + query = h.Rebind(query) + var whes []models.WebhookEvent + if err := h.SelectContext(ctx, &whes, query, args...); err != nil { + return nil, err + } + all = append(all, whes...) + } + return all, nil +} + +// maxWebhooksPerRepo caps the number of webhooks returned per repository to +// prevent unbounded memory use for repos with many configured webhooks. +const maxWebhooksPerRepo = 100 + // GetWebhooksByRepoID implements store.WebhookStore. func (*webhookStore) GetWebhooksByRepoID(ctx context.Context, h db.Handler, repoID int64) ([]models.Webhook, error) { - query := h.Rebind(`SELECT * FROM webhooks WHERE repo_id = ?;`) + query := h.Rebind(`SELECT * FROM webhooks WHERE repo_id = ? LIMIT ?;`) var whs []models.Webhook - err := h.SelectContext(ctx, &whs, query, repoID) + err := h.SelectContext(ctx, &whs, query, repoID, maxWebhooksPerRepo) return whs, err } // GetWebhooksByRepoIDWhereEvent implements store.WebhookStore. func (*webhookStore) GetWebhooksByRepoIDWhereEvent(ctx context.Context, h db.Handler, repoID int64, events []int) ([]models.Webhook, error) { - query, args, err := sqlx.In(`SELECT webhooks.* + query, args, err := sqlx.In(`SELECT DISTINCT webhooks.* FROM webhooks INNER JOIN webhook_events ON webhooks.id = webhook_events.webhook_id WHERE webhooks.repo_id = ? AND webhook_events.event IN (?);`, repoID, events) @@ -149,11 +196,16 @@ func (*webhookStore) GetWebhooksByRepoIDWhereEvent(ctx context.Context, h db.Han return whs, err } +// maxListWebhookDeliveries caps the number of rows returned by +// ListWebhookDeliveriesByWebhookID to prevent unbounded memory use for +// high-volume webhooks. Callers that need more deliveries should paginate. +const maxListWebhookDeliveries = 100 + // ListWebhookDeliveriesByWebhookID implements store.WebhookStore. func (*webhookStore) ListWebhookDeliveriesByWebhookID(ctx context.Context, h db.Handler, webhookID int64) ([]models.WebhookDelivery, error) { - query := h.Rebind(`SELECT id, response_status, event FROM webhook_deliveries WHERE webhook_id = ?;`) + query := h.Rebind(`SELECT id, response_status, event FROM webhook_deliveries WHERE webhook_id = ? ORDER BY created_at DESC LIMIT ?;`) var whds []models.WebhookDelivery - err := h.SelectContext(ctx, &whds, query, webhookID) + err := h.SelectContext(ctx, &whds, query, webhookID, maxListWebhookDeliveries) return whds, err } diff --git a/pkg/store/lfs.go b/pkg/store/lfs.go index 31611e9b5..6221b44a9 100644 --- a/pkg/store/lfs.go +++ b/pkg/store/lfs.go @@ -11,6 +11,7 @@ import ( type LFSStore interface { CreateLFSObject(ctx context.Context, h db.Handler, repoID int64, oid string, size int64) error GetLFSObjectByOid(ctx context.Context, h db.Handler, repoID int64, oid string) (models.LFSObject, error) + GetLFSObjectsByOids(ctx context.Context, h db.Handler, repoID int64, oids []string) ([]models.LFSObject, error) GetLFSObjects(ctx context.Context, h db.Handler, repoID int64) ([]models.LFSObject, error) GetLFSObjectsByName(ctx context.Context, h db.Handler, name string) ([]models.LFSObject, error) DeleteLFSObjectByOid(ctx context.Context, h db.Handler, repoID int64, oid string) error diff --git a/pkg/store/push_mirror.go b/pkg/store/push_mirror.go new file mode 100644 index 000000000..884f4d905 --- /dev/null +++ b/pkg/store/push_mirror.go @@ -0,0 +1,16 @@ +package store + +import ( + "context" + + "github.com/charmbracelet/soft-serve/pkg/db" + "github.com/charmbracelet/soft-serve/pkg/db/models" +) + +// PushMirrorStore is the interface for push mirror storage operations. +type PushMirrorStore interface { + CreatePushMirror(ctx context.Context, tx db.Handler, repoID int64, name, remoteURL string) error + DeletePushMirror(ctx context.Context, tx db.Handler, repoID int64, name string) error + GetPushMirrorsByRepoID(ctx context.Context, tx db.Handler, repoID int64) ([]models.PushMirror, error) + SetPushMirrorEnabled(ctx context.Context, tx db.Handler, repoID int64, name string, enabled bool) error +} diff --git a/pkg/store/store.go b/pkg/store/store.go index 41490cbf8..118809e35 100644 --- a/pkg/store/store.go +++ b/pkg/store/store.go @@ -9,4 +9,5 @@ type Store interface { LFSStore AccessTokenStore WebhookStore + PushMirrorStore } diff --git a/pkg/store/webhooks.go b/pkg/store/webhooks.go index 1c0567b86..b405816ea 100644 --- a/pkg/store/webhooks.go +++ b/pkg/store/webhooks.go @@ -29,6 +29,8 @@ type WebhookStore interface { GetWebhookEventByID(ctx context.Context, h db.Handler, id int64) (models.WebhookEvent, error) // GetWebhookEventsByWebhookID returns all webhook events for a webhook. GetWebhookEventsByWebhookID(ctx context.Context, h db.Handler, webhookID int64) ([]models.WebhookEvent, error) + // GetWebhookEventsByWebhookIDs returns all webhook events for multiple webhooks in one query. + GetWebhookEventsByWebhookIDs(ctx context.Context, h db.Handler, webhookIDs []int64) ([]models.WebhookEvent, error) // CreateWebhookEvents creates webhook events for a webhook. CreateWebhookEvents(ctx context.Context, h db.Handler, webhookID int64, events []int) error // DeleteWebhookEventsByWebhookID deletes all webhook events for a webhook. diff --git a/pkg/sync/workqueue.go b/pkg/sync/workqueue.go index 1c07e74b1..700e51f72 100644 --- a/pkg/sync/workqueue.go +++ b/pkg/sync/workqueue.go @@ -49,7 +49,12 @@ func NewWorkPool(ctx context.Context, workers int, opts ...WorkPoolOption) *Work } // Run starts the workers and waits for them to finish. +// Jobs added via Add while Run is already executing are not guaranteed to be +// picked up by the current invocation — they will be processed on the next +// call to Run. Each job is deleted from the pool when it completes. func (wq *WorkPool) Run() { + var wg sync.WaitGroup + wq.work.Range(func(key, value any) bool { id := key.(string) fn := value.(func()) @@ -58,7 +63,9 @@ func (wq *WorkPool) Run() { return false } + wg.Add(1) go func(id string, fn func()) { + defer wg.Done() defer wq.sem.Release(1) fn() wq.work.Delete(id) @@ -67,18 +74,14 @@ func (wq *WorkPool) Run() { return true }) - if err := wq.sem.Acquire(wq.ctx, int64(wq.workers)); err != nil { - wq.logf("workpool: %v", err) - } + wg.Wait() } // Add adds a new job to the pool. -// If the job already exists, it is a no-op. +// If the job already exists, it is a no-op. LoadOrStore is used to make +// the check-and-store atomic and safe for concurrent callers. func (wq *WorkPool) Add(id string, fn func()) { - if _, ok := wq.work.Load(id); ok { - return - } - wq.work.Store(id, fn) + wq.work.LoadOrStore(id, fn) } // Status checks if a job is in the queue. diff --git a/pkg/task/manager.go b/pkg/task/manager.go index 4f8763711..0ca024f2b 100644 --- a/pkg/task/manager.go +++ b/pkg/task/manager.go @@ -3,6 +3,7 @@ package task import ( "context" "errors" + "fmt" "sync" "sync/atomic" ) @@ -17,12 +18,18 @@ var ( // Task is a task that can be started and stopped. type Task struct { - id string - fn func(context.Context) error - started atomic.Bool - ctx context.Context - cancel context.CancelFunc - err error + id string + fn func(context.Context) error + started atomic.Bool + // completed must be stored only AFTER p.err is written and p.mu is + // unlocked. A concurrent waiter observing completed==true via its own + // p.mu.Lock() is then guaranteed to see the final value of p.err. + // Never move this Store before p.mu.Unlock() or the ordering guarantee breaks. + completed atomic.Bool + ctx context.Context + cancel context.CancelFunc + mu sync.Mutex + err error } // Manager manages tasks. @@ -40,19 +47,30 @@ func NewManager(ctx context.Context) *Manager { } // Add adds a task to the manager. -// If the process already exists, it is a no-op. +// If a task with the same id already exists, Add is a no-op (the existing +// task is not replaced and fn is discarded). +// +// Callers must not call Add for the same id again until they have observed +// either a Stop() call or a completion result from Run(). During the brief +// window after Run() returns (manager-shutdown path) and before the map entry +// is removed by the cleanup goroutine, a subsequent Add will be silently +// dropped. Use Stop() followed by Add() to reliably replace a task. +// +// fn MUST respect context cancellation: when the context passed to fn is +// cancelled (e.g. via Stop or manager shutdown), fn should return promptly. +// A task that ignores cancellation will keep its goroutine alive until fn +// returns naturally — there is no hard deadline imposed by the manager. func (m *Manager) Add(id string, fn func(context.Context) error) { - if m.Exists(id) { - return - } - ctx, cancel := context.WithCancel(m.ctx) - m.m.Store(id, &Task{ + t := &Task{ id: id, fn: fn, ctx: ctx, cancel: cancel, - }) + } + if _, loaded := m.m.LoadOrStore(id, t); loaded { + cancel() + } } // Stop stops the task and removes it from the manager. @@ -75,8 +93,20 @@ func (m *Manager) Exists(id string) bool { return ok } -// Run starts the task if it exists. -// Otherwise, it waits for the process to finish. +// Run starts the task if not already started, or waits for the already-running +// task to complete and delivers its result to done. +// Callers MUST ensure done has capacity >= 1 (buffered) to avoid a panic +// if the caller returns before the task delivers its result. +// +// If Stop() is called while a second goroutine is waiting on an already-started +// task, done receives context.Canceled. Callers should treat context.Canceled as +// "task was stopped or the manager shut down", not necessarily as a task error. +// +// Precondition: Add must have been called for id before Run. If no task is +// registered for id, done receives ErrNotFound and Run returns immediately. +// There is a narrow race window: if Stop() races with Run() between Add and Run, +// Run may return ErrNotFound even though Add was called. Callers should handle +// ErrNotFound gracefully. func (m *Manager) Run(id string, done chan<- error) { v, ok := m.m.Load(id) if !ok { @@ -85,32 +115,87 @@ func (m *Manager) Run(id string, done chan<- error) { } p := v.(*Task) - if p.started.Load() { + if !p.started.CompareAndSwap(false, true) { + // Task is already running (or was already completed). Wait for it. + // Note: between CompareAndSwap returning false and <-p.ctx.Done() + // completing, Stop() may cancel the context and delete the map entry — + // that is safe because p.ctx.Done() still fires and p is still reachable + // through the local pointer. + // Block until the task's context is done. The winner calls p.cancel() + // via defer AFTER writing p.err and storing p.completed=true, so by + // the time <-p.ctx.Done() returns, both writes are visible (Go memory + // model: the defer executes after the preceding statements, providing + // happens-before from the stores to the channel close). <-p.ctx.Done() - if p.err != nil { - done <- p.err + + // Check completed first: if true, the task finished normally and p.err + // holds the authoritative result (nil or non-nil). We must read p.err + // under the mutex to synchronise with the write in the winner path. + if p.completed.Load() { + p.mu.Lock() + err := p.err + p.mu.Unlock() + done <- err return } + // completed is false: p.cancel() was called by Stop() or the manager + // shutting down before the task finished. Return the context error so + // callers can distinguish a premature stop from a task error. done <- p.ctx.Err() + return } - p.started.Store(true) - m.m.Store(id, p) + // We won the CAS: we are the sole goroutine responsible for running p.fn. + // m.m already holds p from Add; no re-store needed here. defer p.cancel() - defer m.m.Delete(id) errc := make(chan error, 1) go func(ctx context.Context) { + // If p.fn panics, the deferred recover fires before errc<-p.fn(ctx) + // produces a value (a panic unwinds the call stack without returning), + // so the recover-path send on errc cannot race with the normal-path send. + // If p.fn returns normally, recover() returns nil and the if is skipped. + defer func() { + if r := recover(); r != nil { + errc <- fmt.Errorf("task panicked: %v", r) + } + }() errc <- p.fn(ctx) }(p.ctx) select { case <-m.ctx.Done(): + // Note: p.cancel() fires via defer AFTER this select arm returns, so + // the p.fn goroutine's ctx is not yet cancelled at this point. + // Callers that observe done's result may briefly see m.Exists(id)==true + // until the background goroutine below drains errc and calls Delete. + // This is an acceptable narrow window and is documented in the API. done <- m.ctx.Err() + // Delay map deletion until the p.fn goroutine has fully exited so that + // a concurrent Add(id, ...) + Run(id, ...) cannot start a new task for + // the same id while the old goroutine is still executing p.fn. + // + // Callers must not re-Add a task with the same id until they have + // observed a Stop() or a completion result from Run(), otherwise the + // new Add() will silently no-op (LoadOrStore sees the stale entry). + go func() { + <-errc + m.m.Delete(id) + }() case err := <-errc: + p.mu.Lock() p.err = err - m.m.Store(id, p) + p.mu.Unlock() + // Mark completed BEFORE p.cancel() fires (via defer) so that any + // concurrent waiter on <-p.ctx.Done() sees completed=true and + // returns nil rather than ctx.Err() for a successfully-finished task. + p.completed.Store(true) + // Deliver the result first, then remove from map. A concurrent + // Run() arriving between Delete and done<-err would get ErrNotFound; + // by deleting after the send, any such caller simply misses this task + // (it has already completed) rather than observing an inconsistent state. done <- err + m.m.Delete(id) } } diff --git a/pkg/ui/pages/repo/diff.go b/pkg/ui/pages/repo/diff.go new file mode 100644 index 000000000..e3e10a3ac --- /dev/null +++ b/pkg/ui/pages/repo/diff.go @@ -0,0 +1,186 @@ +package repo + +import ( + "fmt" + + "charm.land/bubbles/v2/key" + "charm.land/bubbles/v2/spinner" + tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/soft-serve/git" + "github.com/charmbracelet/soft-serve/pkg/proto" + "github.com/charmbracelet/soft-serve/pkg/ui/common" + "github.com/charmbracelet/soft-serve/pkg/ui/components/code" +) + +// DiffMsg is a message sent when the diff content between two refs is loaded. +type DiffMsg struct { + Content string + From string + To string +} + +// Diff is the diff tab component page. +type Diff struct { + common common.Common + code *code.Code + ref *git.Reference + repo proto.Repository + baseRef string + targetRef string + isLoading bool + spinner spinner.Model +} + +// NewDiff creates a new Diff model. +func NewDiff(c common.Common) *Diff { + codeComp := code.New(c, "", ".diff") + codeComp.NoContentStyle = codeComp.NoContentStyle.SetString("No diff available.") + s := spinner.New(spinner.WithSpinner(spinner.Dot), + spinner.WithStyle(c.Styles.Spinner)) + return &Diff{ + common: c, + code: codeComp, + spinner: s, + baseRef: "HEAD~1", + targetRef: "HEAD", + isLoading: true, + } +} + +// Path implements common.TabComponent. +func (d *Diff) Path() string { + return "" +} + +// TabName returns the name of the tab. +func (d *Diff) TabName() string { + return "Diff" +} + +// SetSize implements common.Component. +func (d *Diff) SetSize(width, height int) { + d.common.SetSize(width, height) + d.code.SetSize(width, height) +} + +// ShortHelp implements help.KeyMap. +func (d *Diff) ShortHelp() []key.Binding { + return []key.Binding{ + d.common.KeyMap.UpDown, + } +} + +// FullHelp implements help.KeyMap. +func (d *Diff) FullHelp() [][]key.Binding { + k := d.code.KeyMap + return [][]key.Binding{ + { + k.PageDown, + k.PageUp, + k.HalfPageDown, + k.HalfPageUp, + }, + { + k.Down, + k.Up, + d.common.KeyMap.GotoTop, + d.common.KeyMap.GotoBottom, + }, + } +} + +// Init implements tea.Model. +func (d *Diff) Init() tea.Cmd { + d.isLoading = true + return d.spinner.Tick +} + +// Update implements tea.Model. +func (d *Diff) Update(msg tea.Msg) (common.Model, tea.Cmd) { + cmds := make([]tea.Cmd, 0) + switch msg := msg.(type) { + case RepoMsg: + d.repo = msg + case RefMsg: + d.ref = msg + d.baseRef = "HEAD~1" + d.targetRef = "HEAD" + cmds = append(cmds, d.Init(), d.updateDiffCmd) + case EmptyRepoMsg: + d.ref = nil + d.isLoading = false + cmds = append(cmds, d.code.SetContent("", ".diff")) + case DiffMsg: + d.isLoading = false + d.code.GotoTop() + content := msg.Content + if content == "" { + content = fmt.Sprintf("No differences between %s and %s.", msg.From, msg.To) + } + cmds = append(cmds, d.code.SetContent(content, ".diff")) + case tea.WindowSizeMsg: + d.SetSize(msg.Width, msg.Height) + case spinner.TickMsg: + if d.isLoading && d.spinner.ID() == msg.ID { + s, cmd := d.spinner.Update(msg) + d.spinner = s + if cmd != nil { + cmds = append(cmds, cmd) + } + } + } + c, cmd := d.code.Update(msg) + d.code = c.(*code.Code) + if cmd != nil { + cmds = append(cmds, cmd) + } + return d, tea.Batch(cmds...) +} + +// View implements tea.Model. +func (d *Diff) View() string { + if d.isLoading { + return renderLoading(d.common, d.spinner) + } + return d.code.View() +} + +// SpinnerID implements common.TabComponent. +func (d *Diff) SpinnerID() int { + return d.spinner.ID() +} + +// StatusBarValue implements statusbar.StatusBar. +func (d *Diff) StatusBarValue() string { + return fmt.Sprintf("%s..%s", d.baseRef, d.targetRef) +} + +// StatusBarInfo implements statusbar.StatusBar. +func (d *Diff) StatusBarInfo() string { + return common.ScrollPercent(d.code.ScrollPosition()) +} + +func (d *Diff) updateDiffCmd() tea.Msg { + if d.repo == nil { + return common.ErrorMsg(common.ErrMissingRepo) + } + r, err := d.repo.Open() + if err != nil { + d.common.Logger.Debugf("ui: diff: error opening repository: %v", err) + return DiffMsg{From: d.baseRef, To: d.targetRef} + } + content, err := r.DiffRefs(d.baseRef, d.targetRef) + if err != nil { + d.common.Logger.Debugf("ui: diff: error computing diff %s..%s: %v", d.baseRef, d.targetRef, err) + return DiffMsg{ + Content: fmt.Sprintf("Cannot compute diff %s..%s: %v", d.baseRef, d.targetRef, err), + From: d.baseRef, + To: d.targetRef, + } + } + return DiffMsg{ + Content: content, + From: d.baseRef, + To: d.targetRef, + } +} diff --git a/pkg/ui/pages/repo/files.go b/pkg/ui/pages/repo/files.go index 4c51e3561..4b419a25d 100644 --- a/pkg/ui/pages/repo/files.go +++ b/pkg/ui/pages/repo/files.go @@ -3,7 +3,7 @@ package repo import ( "errors" "fmt" - "path/filepath" + "path" "strings" "charm.land/bubbles/v2/key" @@ -87,12 +87,12 @@ func NewFiles(common common.Common) *Files { lineNumber: true, } selector := selector.New(common, []selector.IdentifiableItem{}, FileItemDelegate{&common}) - selector.SetShowFilter(false) + selector.SetShowFilter(true) selector.SetShowHelp(false) selector.SetShowPagination(false) selector.SetShowStatusBar(false) selector.SetShowTitle(false) - selector.SetFilteringEnabled(false) + selector.SetFilteringEnabled(true) selector.DisableQuitKeybindings() selector.KeyMap.NextPage = common.KeyMap.NextPage selector.KeyMap.PrevPage = common.KeyMap.PrevPage @@ -253,7 +253,7 @@ func (f *Files) Update(msg tea.Msg) (common.Model, tea.Cmd) { switch sel := msg.IdentifiableItem.(type) { case FileItem: f.currentItem = &sel - f.path = filepath.Join(f.path, sel.entry.Name()) + f.path = path.Join(f.path, sel.entry.Name()) if sel.entry.IsTree() { cmds = append(cmds, f.selectTreeCmd) } else { @@ -458,19 +458,19 @@ func (f *Files) selectFileCmd() tea.Msg { if !bin { bin, err = fi.IsBinary() if err != nil { - f.path = filepath.Dir(f.path) + f.path = path.Dir(f.path) return common.ErrorMsg(err) } } if bin { - f.path = filepath.Dir(f.path) + f.path = path.Dir(f.path) return common.ErrorMsg(errBinaryFile) } c, err := fi.Bytes() if err != nil { - f.path = filepath.Dir(f.path) + f.path = path.Dir(f.path) return common.ErrorMsg(err) } @@ -527,7 +527,7 @@ func renderBlame(c common.Common, f *FileItem, b *gitm.Blame) string { } func (f *Files) deselectItemCmd() tea.Cmd { - f.path = filepath.Dir(f.path) + f.path = path.Dir(f.path) index := 0 if len(f.lastSelected) > 0 { index = f.lastSelected[len(f.lastSelected)-1] diff --git a/pkg/ui/pages/repo/log.go b/pkg/ui/pages/repo/log.go index 5a1c177bf..49a2c8f98 100644 --- a/pkg/ui/pages/repo/log.go +++ b/pkg/ui/pages/repo/log.go @@ -118,7 +118,7 @@ func (l *Log) ShortHelp() []key.Binding { } case logViewDiff: copyKey := l.common.KeyMap.Copy - copyKey.SetHelp("c", "copy diff") + copyKey.SetHelp("c", "copy patch") return []key.Binding{ l.common.KeyMap.UpDown, l.common.KeyMap.BackItem, @@ -158,7 +158,7 @@ func (l *Log) FullHelp() [][]key.Binding { }...) case logViewDiff: copyKey := l.common.KeyMap.Copy - copyKey.SetHelp("c", "copy diff") + copyKey.SetHelp("c", "copy patch") k := l.vp.KeyMap b = append(b, []key.Binding{ l.common.KeyMap.BackItem, @@ -259,8 +259,8 @@ func (l *Log) Update(msg tea.Msg) (common.Model, tea.Cmd) { case key.Matches(kmsg, l.common.KeyMap.BackItem): l.goBack() case key.Matches(kmsg, l.common.KeyMap.Copy): - if l.currentDiff != nil { - cmds = append(cmds, copyCmd(l.currentDiff.Patch(), "Commit diff copied to clipboard")) + if l.selectedCommit != nil && l.currentDiff != nil { + cmds = append(cmds, copyCmd(rawPatch(l.selectedCommit, l.currentDiff), "Commit patch copied to clipboard")) } } } @@ -544,3 +544,21 @@ func (l *Log) setItems(items []selector.IdentifiableItem) tea.Cmd { return LogItemsMsg(items) } } + +// rawPatch formats a commit and its diff as an email-format patch, equivalent +// to `git show --format=raw --color=never -p `. +func rawPatch(c *git.Commit, d *git.Diff) string { + var sb strings.Builder + msg := strings.ReplaceAll(c.Message, "\r\n", "\n") + sb.WriteString(fmt.Sprintf("commit %s\n", c.ID.String())) + sb.WriteString(fmt.Sprintf("Author: %s <%s>\n", c.Author.Name, c.Author.Email)) + sb.WriteString(fmt.Sprintf("Date: %s\n\n", c.Author.When.Format(time.UnixDate))) + for _, line := range strings.Split(strings.TrimRight(msg, "\n"), "\n") { + sb.WriteString(" ") + sb.WriteString(line) + sb.WriteByte('\n') + } + sb.WriteByte('\n') + sb.WriteString(d.Patch()) + return sb.String() +} diff --git a/pkg/ui/pages/repo/readme.go b/pkg/ui/pages/repo/readme.go index e7f1b9087..4d7452dd4 100644 --- a/pkg/ui/pages/repo/readme.go +++ b/pkg/ui/pages/repo/readme.go @@ -1,7 +1,7 @@ package repo import ( - "path/filepath" + "path" "charm.land/bubbles/v2/key" "charm.land/bubbles/v2/spinner" @@ -106,6 +106,7 @@ func (r *Readme) Update(msg tea.Msg) (common.Model, tea.Cmd) { case tea.WindowSizeMsg: r.SetSize(msg.Width, msg.Height) case EmptyRepoMsg: + r.isLoading = false cmds = append(cmds, r.code.SetContent(defaultEmptyRepoMsg(r.common.Config(), r.repo.Name()), ".md"), @@ -147,7 +148,7 @@ func (r *Readme) SpinnerID() int { // StatusBarValue implements statusbar.StatusBar. func (r *Readme) StatusBarValue() string { - dir := filepath.Dir(r.readmePath) + dir := path.Dir(r.readmePath) if dir == "." || dir == "" { return " " } @@ -164,7 +165,10 @@ func (r *Readme) updateReadmeCmd() tea.Msg { if r.repo == nil { return common.ErrorMsg(common.ErrMissingRepo) } - rm, rp, _ := backend.Readme(r.repo, r.ref) + rm, rp, err := backend.Readme(r.repo, r.ref) + if err != nil { + return common.ErrorMsg(err) + } m.Content = rm m.Path = rp return m diff --git a/pkg/ui/pages/repo/repo.go b/pkg/ui/pages/repo/repo.go index 58052bd57..7b9200961 100644 --- a/pkg/ui/pages/repo/repo.go +++ b/pkg/ui/pages/repo/repo.go @@ -115,8 +115,11 @@ func (r *Repo) commonHelp() []key.Binding { back.SetHelp("esc", "back to menu") tab := r.common.KeyMap.Section tab.SetHelp("tab", "switch tab") + copyKey := r.common.KeyMap.Copy + copyKey.SetHelp("c", "copy clone cmd") b = append(b, back) b = append(b, tab) + b = append(b, copyKey) return b } @@ -204,6 +207,15 @@ func (r *Repo) Update(msg tea.Msg) (common.Model, tea.Cmd) { switch { case key.Matches(msg, r.common.KeyMap.Back): cmds = append(cmds, goBackCmd) + case key.Matches(msg, r.common.KeyMap.Copy): + // Only handle clone copy on the Readme tab to avoid + // conflicting with pane-level copy handlers (Files, Log, Refs, Stash). + if r.selectedRepo != nil && r.panes[r.activeTab].TabName() == "Readme" { + cmd := r.common.CloneCmd(r.common.Config().SSH.PublicURL, r.selectedRepo.Name()) + if cmd != "" { + cmds = append(cmds, copyCmd(cmd, "Clone command copied to clipboard")) + } + } } } case CopyMsg: @@ -222,6 +234,8 @@ func (r *Repo) Update(msg tea.Msg) (common.Model, tea.Cmd) { cmds = append(cmds, r.updateTabComponent(&Refs{refPrefix: msg.prefix}, msg)) case StashListMsg, StashPatchMsg: cmds = append(cmds, r.updateTabComponent(&Stash{}, msg)) + case DiffMsg: + cmds = append(cmds, r.updateTabComponent(&Diff{}, msg)) // We have two spinners, one is used to when loading the repository and the // other is used when loading the log. // Check if the spinner ID matches the spinner model. diff --git a/pkg/ui/pages/selection/item.go b/pkg/ui/pages/selection/item.go index b6b3a02fa..991582c0e 100644 --- a/pkg/ui/pages/selection/item.go +++ b/pkg/ui/pages/selection/item.go @@ -18,6 +18,9 @@ import ( var _ sort.Interface = Items{} +// copiedResetMsg is sent after the copy feedback timer expires. +type copiedResetMsg struct{ gen int } + // Items is a list of Item. type Items []Item @@ -98,9 +101,11 @@ func (i Item) Command() string { // ItemDelegate is the delegate for the item. type ItemDelegate struct { - common *common.Common - activePane *pane - copiedIdx int + common *common.Common + activePane *pane + copiedItem Item + hasCopied bool + copiedGen int } // NewItemDelegate creates a new ItemDelegate. @@ -108,7 +113,6 @@ func NewItemDelegate(common *common.Common, activePane *pane) *ItemDelegate { return &ItemDelegate{ common: common, activePane: activePane, - copiedIdx: -1, } } @@ -129,7 +133,6 @@ func (d *ItemDelegate) Spacing() int { return 1 } // Update implements list.ItemDelegate. func (d *ItemDelegate) Update(msg tea.Msg, m *list.Model) tea.Cmd { - idx := m.Index() item, ok := m.SelectedItem().(Item) if !ok { return nil @@ -138,12 +141,23 @@ func (d *ItemDelegate) Update(msg tea.Msg, m *list.Model) tea.Cmd { case tea.KeyPressMsg: switch { case key.Matches(msg, d.common.KeyMap.Copy): - d.copiedIdx = idx + d.copiedItem = item + d.hasCopied = true + d.copiedGen++ + gen := d.copiedGen return tea.Batch( tea.SetClipboard(item.Command()), - m.SetItem(idx, item), + func() tea.Msg { + time.Sleep(1500 * time.Millisecond) + return copiedResetMsg{gen: gen} + }, ) } + case copiedResetMsg: + if msg.gen == d.copiedGen { + d.hasCopied = false + d.copiedItem = Item{} + } } return nil } @@ -207,10 +221,9 @@ func (d *ItemDelegate) Render(w io.Writer, m list.Model, index int, listItem lis cmd := i.Command() cmdStyler := styles.Command.Render - if d.copiedIdx == index { + if d.hasCopied && d.copiedItem.ID() == i.ID() { cmd = "(copied to clipboard)" cmdStyler = styles.Desc.Render - d.copiedIdx = -1 } cmd = common.TruncateString(cmd, m.Width()-styles.Base.GetHorizontalFrameSize()) s.WriteString(cmdStyler(cmd)) diff --git a/pkg/ui/pages/selection/selection.go b/pkg/ui/pages/selection/selection.go index 270cf2bb7..983c5398d 100644 --- a/pkg/ui/pages/selection/selection.go +++ b/pkg/ui/pages/selection/selection.go @@ -10,6 +10,7 @@ import ( "charm.land/lipgloss/v2" "github.com/charmbracelet/soft-serve/pkg/access" "github.com/charmbracelet/soft-serve/pkg/backend" + "github.com/charmbracelet/soft-serve/pkg/proto" "github.com/charmbracelet/soft-serve/pkg/ui/common" "github.com/charmbracelet/soft-serve/pkg/ui/components/code" "github.com/charmbracelet/soft-serve/pkg/ui/components/selector" @@ -191,7 +192,8 @@ func (s *Selection) Init() tea.Cmd { ctx := s.common.Context() be := s.common.Backend() pk := s.common.PublicKey() - if pk == nil && !be.AllowKeyless(ctx) { + user := proto.UserFromContext(ctx) + if pk == nil && user == nil && !be.AllowKeyless(ctx) { return nil } @@ -203,7 +205,7 @@ func (s *Selection) Init() tea.Cmd { for _, r := range repos { if r.Name() == ".soft-serve" { readme, path, err := backend.Readme(r, nil) - if err != nil { + if err != nil || path == "" { continue } @@ -213,7 +215,7 @@ func (s *Selection) Init() tea.Cmd { if r.IsHidden() { continue } - al := be.AccessLevelByPublicKey(ctx, r.Name(), pk) + al := be.AccessLevelForUser(ctx, r.Name(), proto.UserFromContext(ctx)) if al >= access.ReadOnlyAccess { item, err := NewItem(s.common, r) if err != nil { diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 2fb66e55d..5efd29111 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -5,6 +5,7 @@ import ( "path" "strings" "unicode" + "unicode/utf8" "github.com/charmbracelet/x/ansi" ) @@ -34,7 +35,8 @@ func ValidateUsername(username string) error { return fmt.Errorf("username cannot be empty") } - if !unicode.IsLetter(rune(username[0])) { + first, _ := utf8.DecodeRuneInString(username) + if !unicode.IsLetter(first) { return fmt.Errorf("username must start with a letter") } diff --git a/pkg/web/auth.go b/pkg/web/auth.go index 86e15f1ef..3c8f301e3 100644 --- a/pkg/web/auth.go +++ b/pkg/web/auth.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "strings" + "time" "charm.land/log/v2" "github.com/charmbracelet/soft-serve/pkg/backend" @@ -167,5 +168,42 @@ func parseJWT(ctx context.Context, bearer string) (*jwt.RegisteredClaims, error) return nil, ErrInvalidToken } + // Validate JWT claims before accepting. + // Prevents not-before, issuer, and audience attacks. + if err := validateJWTClaims(ctx, cfg, claims); err != nil { + return nil, ErrInvalidToken + } + return claims, nil } + +// validateJWTClaims validates JWT claims for security. +// Prevents not-before, issuer, and audience attacks. +func validateJWTClaims(ctx context.Context, cfg *config.Config, claims *jwt.RegisteredClaims) error { + // Validate expiration time if set + if claims.ExpiresAt != nil && claims.ExpiresAt.Time.Before(time.Now()) { + return errors.New("token expired") + } + + // Validate not-before time if set + if claims.NotBefore != nil && claims.NotBefore.Time.After(time.Now()) { + return errors.New("token not yet valid") + } + + // Validate issuer + if claims.Issuer != cfg.HTTP.PublicURL { + return errors.New("invalid token issuer") + } + + // Validate audience - audience is optional but if set must match public URL + // jwt.ClaimStrings is a slice of strings + if len(claims.Audience) > 0 { + for _, audience := range claims.Audience { + if !strings.HasPrefix(audience, cfg.HTTP.PublicURL) { + return errors.New("invalid token audience") + } + } + } + + return nil +} diff --git a/pkg/web/blob.go b/pkg/web/blob.go new file mode 100644 index 000000000..955baa9b4 --- /dev/null +++ b/pkg/web/blob.go @@ -0,0 +1,203 @@ +package web + +import ( + "bytes" + "mime" + "net/http" + "net/url" + "path/filepath" + "strconv" + "strings" + + gitb "github.com/charmbracelet/soft-serve/git" + "github.com/gorilla/mux" +) + +// maxRawBlobSize is the largest blob that getRawBlob will read into memory. +// Requests for blobs exceeding this size receive HTTP 413. +const maxRawBlobSize = 32 * 1024 * 1024 // 32 MiB + +// getRawBlob serves the raw content of a single file at a given ref and path. +// It is registered as GET /{repo}/raw/{ref}/{filepath}. +// +// Access control is enforced by the withAccess middleware that wraps this +// handler: unauthenticated users see 401, insufficient-access users see 404 +// (to avoid leaking repo existence). The handler never re-checks access. +// +// The Accept header controls delivery: +// - "application/octet-stream" → Content-Disposition: attachment (download) +// - anything else → Content-Type inferred from extension or +// binary detection (text/plain for text, application/octet-stream for binary) +// +// Note: dir is constructed and sanitised by the withParams middleware and must +// not be derived locally inside this handler. +func getRawBlob(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + dir := vars["dir"] + ref := vars["ref"] + filePath := vars["filepath"] + + if filePath == "" { + renderBadRequest(w, r) + return + } + + repo, err := gitb.Open(dir) + if err != nil { + renderNotFound(w, r) + return + } + + // Resolve HEAD when no ref is given. + if ref == "" || ref == "HEAD" { + head, err := repo.HEAD() + if err != nil { + renderNotFound(w, r) + return + } + ref = head.ID + } + + tree, err := repo.LsTree(ref) + if err != nil { + renderNotFound(w, r) + return + } + + te, err := tree.TreeEntry(filePath) + if err != nil { + renderNotFound(w, r) + return + } + + // Must be a blob (file), not a tree (directory). + if te.Type() != "blob" { + renderNotFound(w, r) + return + } + + // Guard against OOM/DoS from very large blobs. + // te.Size() calls `git cat-file -s` and silently returns 0 on error, + // so this is a fast early-out only; the post-load check below is the + // authoritative guard. + if te.Size() > maxRawBlobSize { + renderStatus(http.StatusRequestEntityTooLarge)(w, r) + return + } + + bts, err := te.Contents() + if err != nil { + renderInternalServerError(w, r) + return + } + + // Belt-and-suspenders: re-check after loading in case te.Size() returned + // 0 due to a silent git subprocess error. + if int64(len(bts)) > maxRawBlobSize { + renderStatus(http.StatusRequestEntityTooLarge)(w, r) + return + } + + // Determine Content-Type from extension first, then fall back to binary + // detection using the bytes already in memory (avoids a second git subprocess). + contentType := mime.TypeByExtension(filepath.Ext(filePath)) + if contentType == "" { + isBin, _ := gitb.IsBinary(bytes.NewReader(bts)) + if isBin { + contentType = "application/octet-stream" + } else { + contentType = "text/plain; charset=utf-8" + } + } + + // Sanitise: downgrade any MIME type that a browser will execute scripts from. + // This prevents stored-XSS when an attacker pushes an .html/.svg/.js file. + contentType = sanitizeMIME(contentType) + + // X-Content-Type-Options prevents browsers from sniffing and upgrading the type. + w.Header().Set("X-Content-Type-Options", "nosniff") + + // If the client explicitly requests a binary stream, serve as download. + // NOTE: sanitizeMIME has already run above. Any content-type set in this + // block must itself be safe to serve without further sanitisation. + if r.Header.Get("Accept") == "application/octet-stream" { + contentType = "application/octet-stream" + // Build a safe filename for Content-Disposition per RFC 6266 §4.3 and + // RFC 5987: strip ASCII control characters to prevent header injection, + // then emit both filename= (ASCII fallback) and filename*= (RFC 5987) + // forms so all clients get the correct name. + rawName := filepath.Base(filePath) + safeName := strings.Map(func(r rune) rune { + if r < 0x20 || r == 0x7F { + return -1 // drop control character + } + return r + }, rawName) + asciiName := strings.ReplaceAll(safeName, `"`, `\"`) + // Check whether the name is pure ASCII (no code point above 0x7E). + isASCII := true + for _, r := range safeName { + if r > 0x7E { + isASCII = false + break + } + } + if isASCII { + w.Header().Set("Content-Disposition", `attachment; filename="`+asciiName+`"`) + } else { + // RFC 5987 encoded filename: UTF-8''. + // url.PathEscape percent-encodes all non-unreserved characters except '/'. + // Replace any literal '/' that might appear in a segment (shouldn't + // after filepath.Base, but be safe). + encoded := strings.ReplaceAll(url.PathEscape(safeName), "/", "%2F") + w.Header().Set("Content-Disposition", + `attachment; filename="`+asciiName+`"; filename*=UTF-8''`+encoded) + } + } + + // Mutable refs (branch names, tags) must not be cached by proxies. + // A future improvement could set max-age for immutable SHA refs. + w.Header().Set("Cache-Control", "no-store") + + w.Header().Set("Content-Type", contentType) + w.Header().Set("Content-Length", strconv.FormatInt(int64(len(bts)), 10)) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(bts) +} + +// sanitizeMIME uses an allowlist to ensure only MIME types that cannot execute +// scripts in a browser are forwarded. Everything else is downgraded to +// text/plain to prevent stored-XSS from pushed .html/.svg/.js/.css/etc. files. +// +// Allowlisted categories: +// - text/plain, application/octet-stream (safe by definition) +// - application/json (data only, not rendered/executed by browsers) +// - image/* except SVG (SVG allows embedded scripts) +// - audio/*, video/* (media; cannot execute scripts) +// +// All other types — including text/html, text/css, *+xml, */javascript, +// application/pdf, font/*, multipart/* — are downgraded. +func sanitizeMIME(ct string) string { + // Strip parameters for comparison (e.g. "text/html; charset=utf-8" → "text/html"). + base := ct + if i := strings.Index(ct, ";"); i != -1 { + base = strings.TrimSpace(ct[:i]) + } + base = strings.ToLower(base) + + switch { + case base == "text/plain", + base == "application/octet-stream", + base == "application/json": + return ct + case strings.HasPrefix(base, "image/") && base != "image/svg+xml": + // SVG is excluded: it supports embedded