From d1803eb10ee3ca8069c13f9d026e2c378f998db5 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 10:23:18 +0100 Subject: [PATCH 001/148] feat: add agents configuration --- AGENTS.md | 235 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 AGENTS.md 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. From 4eb161d50e2bf5c6a72cf4a5ff8da3eff8428840 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 13:38:18 +0100 Subject: [PATCH 002/148] fix(ssh): validate access tokens in keyboard-interactive auth (#800) Access tokens were not providing authentication because: 1. KeyboardInteractiveHandler ignored the challenge callback entirely, never prompting for or validating tokens. 2. SSH commands used UserByPublicKey/AccessLevelByPublicKey with no fallback to the context user, so token sessions got no identity. Changes: - Rewrite KeyboardInteractiveHandler to prompt for an access token, validate it via UserByAccessToken, and store the username in the permissions extensions under "token-user" key. - Update AuthenticationMiddleware to resolve token-authenticated users from the "token-user" extension and set them in context. - Add currentUser() helper in cmd.go that resolves user by public key first, then falls back to context user (for token sessions). - Update info, pubkey, set_username commands to use currentUser(). - Fix N+1 query in repo list and TUI selection by using context user instead of per-repo public key lookups. - Add nil guard to AccessLevelByPublicKey with context-user fallback. - Add integration tests covering all token auth scenarios. Fixes charmbracelet/soft-serve#800 Co-Authored-By: Claude Opus 4.6 --- pkg/backend/user.go | 19 +++++--- pkg/ssh/cmd/cmd.go | 16 +++++++ pkg/ssh/cmd/info.go | 3 +- pkg/ssh/cmd/list.go | 6 +-- pkg/ssh/cmd/pubkey.go | 9 ++-- pkg/ssh/cmd/set_username.go | 4 +- pkg/ssh/middleware.go | 17 +++++-- pkg/ssh/session.go | 2 +- pkg/ssh/ssh.go | 36 ++++++++++---- pkg/ui/pages/selection/selection.go | 3 +- testscript/script_test.go | 43 +++++++++++++++++ testscript/testdata/token-ssh-auth.txtar | 61 ++++++++++++++++++++++++ 12 files changed, 186 insertions(+), 33 deletions(-) create mode 100644 testscript/testdata/token-ssh-auth.txtar diff --git a/pkg/backend/user.go b/pkg/backend/user.go index 736842f9f..75335fc4e 100644 --- a/pkg/backend/user.go +++ b/pkg/backend/user.go @@ -27,15 +27,22 @@ func (d *Backend) AccessLevel(ctx context.Context, repo string, username string) // // 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 + if pk != nil { + 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()) } } - user, _ := d.UserByPublicKey(ctx, pk) - if user != nil { - return d.AccessLevel(ctx, repo, user.Username()) + // Fall back to the context user (e.g., authenticated via access token). + if ctxUser := proto.UserFromContext(ctx); ctxUser != nil { + return d.AccessLevel(ctx, repo, ctxUser.Username()) } return d.AccessLevel(ctx, repo, "") diff --git a/pkg/ssh/cmd/cmd.go b/pkg/ssh/cmd/cmd.go index 18624eb33..43082d2e8 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,21 @@ func CommandName(args []string) string { return args[0] } +// currentUser resolves the authenticated user from the session context. +// It first tries public key auth, then falls back to the context user +// (e.g., set by access token authentication). +func currentUser(ctx context.Context, be *backend.Backend) (proto.User, error) { + pk := sshutils.PublicKeyFromContext(ctx) + if pk != nil { + return be.UserByPublicKey(ctx, pk) + } + 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/info.go b/pkg/ssh/cmd/info.go index bb932390a..af52a4f88 100644 --- a/pkg/ssh/cmd/info.go +++ b/pkg/ssh/cmd/info.go @@ -15,8 +15,7 @@ func InfoCommand() *cobra.Command { 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, be) 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..8acc0a811 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, be) 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, be) if err != nil { return err } @@ -68,8 +66,7 @@ func PubkeyCommand() *cobra.Command { 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, be) if err != nil { return err } diff --git a/pkg/ssh/cmd/set_username.go b/pkg/ssh/cmd/set_username.go index 1fe93616b..c7ad1aa05 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, be) if err != nil { return err } diff --git a/pkg/ssh/middleware.go b/pkg/ssh/middleware.go index 25a00b1a2..1c48c5d6a 100644 --- a/pkg/ssh/middleware.go +++ b/pkg/ssh/middleware.go @@ -55,17 +55,28 @@ func AuthenticationMiddleware(sh ssh.Handler) ssh.Handler { return } + // Check if this session was authenticated via access token. + tokenUser := perms.Extensions[tokenUserExtKey] + ac := be.AllowKeyless(ctx) - publicKeyCounter.WithLabelValues(strconv.FormatBool(ac || pk != nil)).Inc() - if !ac && pk == nil { + publicKeyCounter.WithLabelValues(strconv.FormatBool(ac || pk != nil || tokenUser != "")).Inc() + if !ac && pk == nil && tokenUser == "" { wish.Fatalln(s, ErrPermissionDenied) return } - // Set the auth'd user, or anon, in the context + // Resolve the authenticated user identity. var user proto.User if pk != nil { user, _ = be.UserByPublicKey(ctx, pk) + } else if tokenUser != "" { + var err error + user, err = be.User(ctx, tokenUser) + if err != nil { + log.FromContext(ctx).Warn("token-authenticated user not found", "username", tokenUser, "err", err) + wish.Fatalln(s, ErrPermissionDenied) + return + } } ctx.SetValue(proto.ContextKeyUser, user) 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..1d3a81cfc 100644 --- a/pkg/ssh/ssh.go +++ b/pkg/ssh/ssh.go @@ -23,6 +23,10 @@ import ( gossh "golang.org/x/crypto/ssh" ) +// tokenUserExtKey is the permissions extension key used to pass the +// authenticated username from KeyboardInteractiveHandler to AuthenticationMiddleware. +const tokenUserExtKey = "token-user" + var ( publicKeyCounter = promauto.NewCounterVec(prometheus.CounterOpts{ Namespace: "soft_serve", @@ -180,18 +184,34 @@ func (s *SSHServer) PublicKeyHandler(ctx ssh.Context, pk ssh.PublicKey) (allowed } // 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 && len(answers) > 0 && answers[0] != "" { + token := answers[0] + user, tokenErr := s.be.UserByAccessToken(ctx, token) + if tokenErr == nil && user != nil { + // Valid token: store user identity and allow access. + perms.Extensions["pubkey-fp"] = "" + perms.Extensions[tokenUserExtKey] = user.Username() + ctx.SetValue(ssh.ContextKeyPermissions, perms) + keyboardInteractiveCounter.WithLabelValues("true").Inc() + s.logger.Info("keyboard-interactive token auth succeeded", "username", user.Username()) + return true + } + s.logger.Warn("keyboard-interactive token auth failed", "err", tokenErr) + } + + // 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) } diff --git a/pkg/ui/pages/selection/selection.go b/pkg/ui/pages/selection/selection.go index 270cf2bb7..e66a8d552 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" @@ -213,7 +214,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/testscript/script_test.go b/testscript/script_test.go index 550a3b3e0..68e96aed2 100644 --- a/testscript/script_test.go +++ b/testscript/script_test.go @@ -95,6 +95,7 @@ func TestScript(t *testing.T) { "soft": cmdSoft("admin", admin1.Signer()), "usoft": cmdSoft("user1", user1.Signer()), "attacksoft": cmdSoft("attacker", attackerSigner, attacker.Signer()), + "tokensoft": cmdTokenSoft, "git": cmdGit(admin1Key), "ugit": cmdGit(user1Key), "agit": cmdGit(attackerKey), @@ -221,6 +222,48 @@ func cmdSoft(user string, keys ...ssh.Signer) func(ts *testscript.TestScript, ne } } +// cmdTokenSoft connects via keyboard-interactive auth using an access token. +// Usage: tokensoft +func cmdTokenSoft(ts *testscript.TestScript, neg bool, args []string) { + if len(args) < 2 { + ts.Fatalf("usage: tokensoft ") + } + token := args[0] + cmdArgs := args[1:] + + cli, err := ssh.Dial( + "tcp", + net.JoinHostPort("localhost", ts.Getenv("SSH_PORT")), + &ssh.ClientConfig{ + User: "git", + Auth: []ssh.AuthMethod{ + ssh.KeyboardInteractive(func(name, instruction string, questions []string, echos []bool) ([]string, error) { + answers := make([]string, len(questions)) + for i := range questions { + answers[i] = token + } + return answers, nil + }), + }, + HostKeyCallback: ssh.InsecureIgnoreHostKey(), + }, + ) + if neg && err != nil { + return + } + ts.Check(err) + defer cli.Close() + + sess, err := cli.NewSession() + ts.Check(err) + defer sess.Close() + + sess.Stdout = ts.Stdout() + sess.Stderr = ts.Stderr() + + check(ts, sess.Run(strings.Join(cmdArgs, " ")), neg) +} + func cmdUI(key ssh.Signer) func(ts *testscript.TestScript, neg bool, args []string) { return func(ts *testscript.TestScript, neg bool, args []string) { if len(args) < 1 { diff --git a/testscript/testdata/token-ssh-auth.txtar b/testscript/testdata/token-ssh-auth.txtar new file mode 100644 index 000000000..ab23fb07c --- /dev/null +++ b/testscript/testdata/token-ssh-auth.txtar @@ -0,0 +1,61 @@ +# vi: set ft=conf +# Test that SSH keyboard-interactive auth validates access tokens +# and resolves user identity for collaborator-based access control. +# +# Regression test for: https://github.com/charmbracelet/soft-serve/issues/800 + +# start soft serve +exec soft serve & +# wait for SSH server to start +ensureserverrunning SSH_PORT + +# create a user with a public key +soft user create user1 --key "$USER1_AUTHORIZED_KEY" + +# create an access token for user1 +usoft token create 'testtoken' +stdout 'ss_.*' +cp stdout token.txt +envfile TOKEN=token.txt + +# TEST 1: Valid token authenticates successfully with AllowKeyless=false +soft settings allow-keyless false +tokensoft $TOKEN repo list + +# TEST 2: Invalid token is rejected with AllowKeyless=false +! tokensoft invalid-token-value repo list + +# TEST 3: Valid token works with AllowKeyless=true +soft settings allow-keyless true +tokensoft $TOKEN repo list + +# TEST 4: Token-authenticated session has user identity (collaborator access) +# Create a private repo only accessible to collaborators +soft repo create private-repo -p +soft repo collab add private-repo user1 + +# Connect with valid token — user1 should see the private repo +soft settings allow-keyless false +tokensoft $TOKEN repo list +stdout 'private-repo' + +# TEST 5: Invalid token with AllowKeyless=true gets anonymous access (no private repo) +soft settings allow-keyless true +tokensoft invalid-token-value repo list +! stdout 'private-repo' + +# TEST 6: info command works with token auth +soft settings allow-keyless false +tokensoft $TOKEN info +stdout 'Username: user1' + +# TEST 7: pubkey list works with token auth +tokensoft $TOKEN pubkey list +stdout 'ssh-' + +# TEST 8: token list works with token auth (uses UserFromContext, already correct) +tokensoft $TOKEN token list +stdout 'testtoken' + +# stop the server +[windows] stopserver From a4d3f655bbd8a19a179c0d1ec3c929460aad7951 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 14:29:37 +0100 Subject: [PATCH 003/148] fix(ssh): accept split key args in user create -k (#750) When passing a public key via SSH, the SSH layer splits the command string on whitespace. A key like "ssh-ed25519 AAAA..." becomes two separate tokens, so cobra received the key type as the flag value and the base64 blob as an extra positional arg, failing ExactArgs(1). Fix by accepting extra positional args after the username and joining them as the key value when -k is not provided. This mirrors the pattern already used by user add-pubkey. Fixes charmbracelet/soft-serve#750 Co-Authored-By: Claude Opus 4.6 --- pkg/ssh/cmd/user.go | 9 ++++-- testscript/testdata/user-create-key.txtar | 38 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 testscript/testdata/user-create-key.txtar diff --git a/pkg/ssh/cmd/user.go b/pkg/ssh/cmd/user.go index 21981f9cb..47aa0e3d3 100644 --- a/pkg/ssh/cmd/user.go +++ b/pkg/ssh/cmd/user.go @@ -22,15 +22,20 @@ func UserCommand() *cobra.Command { var admin bool var key string userCreateCommand := &cobra.Command{ - Use: "create USERNAME", + Use: "create USERNAME [AUTHORIZED_KEY]", Short: "Create a new user", - Args: cobra.ExactArgs(1), + 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] + // Accept key as remaining positional args (joined) to handle the case + // where the SSH layer splits "ssh-ed25519 AAAA..." into two tokens. + if key == "" && len(args) > 1 { + key = strings.Join(args[1:], " ") + } if key != "" { pk, _, err := sshutils.ParseAuthorizedKey(key) if err != nil { diff --git a/testscript/testdata/user-create-key.txtar b/testscript/testdata/user-create-key.txtar new file mode 100644 index 000000000..12cf59492 --- /dev/null +++ b/testscript/testdata/user-create-key.txtar @@ -0,0 +1,38 @@ +# vi: set ft=conf + +# Regression test for https://github.com/charmbracelet/soft-serve/issues/750 +# user create -k "ssh-ed25519 AAAA..." should work even though the key +# contains a space which the SSH layer splits into multiple tokens. + +# start soft serve +exec soft serve & +# wait for SSH server to start +ensureserverrunning SSH_PORT + +# TEST 1: user create with -k flag (key has a space — split across SSH args) +soft user create alice -k "$USER1_AUTHORIZED_KEY" +soft user info alice +stdout 'Username: alice' +stdout 'ssh-ed25519' + +# TEST 2: user alice can connect using her key +usoft info +stdout 'Username: alice' + +# TEST 3: user create with a different key works (no duplicate constraint) +soft user create bob -k "$ADMIN2_AUTHORIZED_KEY" +soft user info bob +stdout 'Username: bob' +stdout 'ssh-ed25519' + +# TEST 4: user create without any key still works +soft user create charlie +soft user info charlie +stdout 'Username: charlie' + +# TEST 5: invalid key is rejected +! soft user create dave -k 'not-a-valid-key' +stderr '.' + +# stop the server +[windows] stopserver From faaa55b2ae5c2b93f6269c7246c43d813276db7e Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 14:41:44 +0100 Subject: [PATCH 004/148] feat(backend): set gitweb.owner on repo create (#753) When a repository is created by an authenticated user, set the gitweb.owner git config variable to the creator's username. This allows gitweb and other tools that read gitweb.owner to display the correct repository owner. Fixes charmbracelet/soft-serve#753 Co-Authored-By: Claude Opus 4.6 --- pkg/backend/repo.go | 15 ++++++++++++++- testscript/testdata/gitweb-owner.txtar | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 testscript/testdata/gitweb-owner.txtar diff --git a/pkg/backend/repo.go b/pkg/backend/repo.go index f9b70bbc3..69027246a 100644 --- a/pkg/backend/repo.go +++ b/pkg/backend/repo.go @@ -65,12 +65,25 @@ func (d *Backend) CreateRepository(ctx context.Context, name string, user proto. return err } - _, err := git.Init(rp, true) + 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 { d.logger.Error("failed to write description", "repo", name, "err", err) return err diff --git a/testscript/testdata/gitweb-owner.txtar b/testscript/testdata/gitweb-owner.txtar new file mode 100644 index 000000000..bc1e03a3c --- /dev/null +++ b/testscript/testdata/gitweb-owner.txtar @@ -0,0 +1,25 @@ +# vi: set ft=conf + +# Regression test for https://github.com/charmbracelet/soft-serve/issues/753 +# gitweb.owner should be set in the git config when a repo is created. + +# start soft serve +exec soft serve & +# wait for SSH server to start +ensureserverrunning SSH_PORT + +# create user1 (non-admin) +soft user create user1 --key "$USER1_AUTHORIZED_KEY" + +# TEST 1: admin creates a repo — gitweb.owner should be "admin" +soft repo create adminrepo +readfile $DATA_PATH/repos/adminrepo.git/config +stdout 'owner = admin' + +# TEST 2: user1 creates a repo — gitweb.owner should be "user1" +usoft repo create userrepo +readfile $DATA_PATH/repos/userrepo.git/config +stdout 'owner = user1' + +# stop the server +[windows] stopserver From 4a53343ca85e1a1dd9d52914978dd1e292167756 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 17:06:25 +0100 Subject: [PATCH 005/148] feat(ui): add 'c' shortcut to copy clone command in repo view (#769) When viewing a repository in the TUI, pressing 'c' now copies the git clone command to the clipboard. Previously this shortcut was only available from the repository list, not inside a repo view. Fixes charmbracelet/soft-serve#769 Co-Authored-By: Claude Opus 4.6 --- pkg/ui/pages/repo/repo.go | 8 ++++++++ testscript/testdata/ui-repo-copy.txtar | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 testscript/testdata/ui-repo-copy.txtar diff --git a/pkg/ui/pages/repo/repo.go b/pkg/ui/pages/repo/repo.go index 58052bd57..c0b384d67 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") + copy := r.common.KeyMap.Copy + copy.SetHelp("c", "copy clone cmd") b = append(b, back) b = append(b, tab) + b = append(b, copy) return b } @@ -204,6 +207,11 @@ 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): + if r.selectedRepo != nil { + cmd := r.common.CloneCmd(r.common.Config().SSH.PublicURL, r.selectedRepo.Name()) + cmds = append(cmds, copyCmd(cmd, "Command copied to clipboard")) + } } } case CopyMsg: diff --git a/testscript/testdata/ui-repo-copy.txtar b/testscript/testdata/ui-repo-copy.txtar new file mode 100644 index 000000000..8b5f81e7d --- /dev/null +++ b/testscript/testdata/ui-repo-copy.txtar @@ -0,0 +1,26 @@ +# vi: set ft=conf + +# Test for https://github.com/charmbracelet/soft-serve/issues/769 +# The 'c' shortcut to copy the clone command should be available +# when viewing a repository in the TUI. + +# start soft serve +exec soft serve & +ensureserverrunning SSH_PORT + +# create a repo and push a commit so it's not empty +soft repo create testrepo +git clone ssh://localhost:$SSH_PORT/testrepo repo +mkfile ./repo/README.md '# Test' +git -C repo add -A +git -C repo commit -m 'init' +git -C repo push origin HEAD + +# open TUI, navigate into testrepo (enter), expand help (?), wait, quit +# key sequence: wait, enter to select repo, wait, ? to expand help, wait, quit +ui '" \r ? q"' +cp stdout repo-view.txt +grep 'copy clone cmd' repo-view.txt + +# stop the server +[windows] stopserver From 1ec1950b7ed1d9d7365dc26e81d743abe608db1f Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 19:24:22 +0100 Subject: [PATCH 006/148] fix(ssh): merge trailing args into key unconditionally When SSH splits -k "ssh-ed25519 AAAA...", cobra may capture the first token into the -k flag value with the rest as positional args. The previous key == "" guard missed this case. Now always merge trailing args into the key value, matching the add-pubkey pattern. Also reverts Use string to "create USERNAME" to avoid advertising an unintended positional-key API. Co-Authored-By: Claude Opus 4.6 --- pkg/ssh/cmd/user.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/ssh/cmd/user.go b/pkg/ssh/cmd/user.go index 47aa0e3d3..a72f8af12 100644 --- a/pkg/ssh/cmd/user.go +++ b/pkg/ssh/cmd/user.go @@ -22,7 +22,7 @@ func UserCommand() *cobra.Command { var admin bool var key string userCreateCommand := &cobra.Command{ - Use: "create USERNAME [AUTHORIZED_KEY]", + Use: "create USERNAME", Short: "Create a new user", Args: cobra.MinimumNArgs(1), PersistentPreRunE: checkIfAdmin, @@ -31,10 +31,13 @@ func UserCommand() *cobra.Command { ctx := cmd.Context() be := backend.FromContext(ctx) username := args[0] - // Accept key as remaining positional args (joined) to handle the case - // where the SSH layer splits "ssh-ed25519 AAAA..." into two tokens. - if key == "" && len(args) > 1 { - key = strings.Join(args[1:], " ") + // When -k is passed over SSH, the key value may be split across + // the flag and trailing positional args (e.g. -k "ssh-ed25519 AAAA" + // becomes -k ssh-ed25519 AAAA as separate tokens). Merge any + // trailing args into the key to reassemble the full value. + if len(args) > 1 { + key = strings.Join(append([]string{key}, args[1:]...), " ") + key = strings.TrimSpace(key) } if key != "" { pk, _, err := sshutils.ParseAuthorizedKey(key) From 2753b7136854ee2bdf8a2ba08bb99b773408c2ae Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 19:28:48 +0100 Subject: [PATCH 007/148] fix(ui): prevent double-copy and empty clipboard on 'c' key - Only trigger clone command copy on Readme tab to avoid conflicting with pane-level copy handlers (Files, Log, Refs, Stash each have their own 'c' handler for copying file names, commit hashes, etc.) - Guard against empty clone command when HideCloneCmd is true - Use more specific status bar message "Clone command copied" Co-Authored-By: Claude Opus 4.6 --- pkg/ui/pages/repo/repo.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/ui/pages/repo/repo.go b/pkg/ui/pages/repo/repo.go index c0b384d67..6d52d5511 100644 --- a/pkg/ui/pages/repo/repo.go +++ b/pkg/ui/pages/repo/repo.go @@ -208,9 +208,13 @@ func (r *Repo) Update(msg tea.Msg) (common.Model, tea.Cmd) { case key.Matches(msg, r.common.KeyMap.Back): cmds = append(cmds, goBackCmd) case key.Matches(msg, r.common.KeyMap.Copy): - if r.selectedRepo != nil { + // 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()) - cmds = append(cmds, copyCmd(cmd, "Command copied to clipboard")) + if cmd != "" { + cmds = append(cmds, copyCmd(cmd, "Clone command copied to clipboard")) + } } } } From 8cdd0a70b376efbc7baff3eaecc6e90b2e5ff87c Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 19:30:21 +0100 Subject: [PATCH 008/148] fix(ssh): improve token auth logging and remove redundant DB lookup - Fix misleading log when token resolves to nil user (logged err=, now logs err="user not found") - Simplify currentUser() to use context user directly since AuthenticationMiddleware already resolves the user for both public key and token auth paths Co-Authored-By: Claude Opus 4.6 --- pkg/ssh/cmd/cmd.go | 10 +++------- pkg/ssh/ssh.go | 6 +++++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/ssh/cmd/cmd.go b/pkg/ssh/cmd/cmd.go index 43082d2e8..25cd21ada 100644 --- a/pkg/ssh/cmd/cmd.go +++ b/pkg/ssh/cmd/cmd.go @@ -111,13 +111,9 @@ func CommandName(args []string) string { } // currentUser resolves the authenticated user from the session context. -// It first tries public key auth, then falls back to the context user -// (e.g., set by access token authentication). -func currentUser(ctx context.Context, be *backend.Backend) (proto.User, error) { - pk := sshutils.PublicKeyFromContext(ctx) - if pk != nil { - return be.UserByPublicKey(ctx, pk) - } +// 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, _ *backend.Backend) (proto.User, error) { user := proto.UserFromContext(ctx) if user != nil { return user, nil diff --git a/pkg/ssh/ssh.go b/pkg/ssh/ssh.go index 1d3a81cfc..148beb841 100644 --- a/pkg/ssh/ssh.go +++ b/pkg/ssh/ssh.go @@ -204,7 +204,11 @@ func (s *SSHServer) KeyboardInteractiveHandler(ctx ssh.Context, challenge gossh. s.logger.Info("keyboard-interactive token auth succeeded", "username", user.Username()) return true } - s.logger.Warn("keyboard-interactive token auth failed", "err", tokenErr) + 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. From 6562765c59cccb1aa0e27653a694a4cd79a37faa Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 19:48:25 +0100 Subject: [PATCH 009/148] fix(ssh): reject extra args when -k is not provided When -k is not provided but extra positional args are present (e.g. user create alice bob), return a clear error instead of silently treating them as key material. Co-Authored-By: Claude Opus 4.6 --- pkg/ssh/cmd/user.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/ssh/cmd/user.go b/pkg/ssh/cmd/user.go index a72f8af12..0e7b03c7a 100644 --- a/pkg/ssh/cmd/user.go +++ b/pkg/ssh/cmd/user.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "sort" "strings" @@ -36,6 +37,9 @@ func UserCommand() *cobra.Command { // becomes -k ssh-ed25519 AAAA as separate tokens). Merge any // trailing args into the key to reassemble the full value. if len(args) > 1 { + if key == "" { + return fmt.Errorf("accepts 1 arg(s), received %d", len(args)) + } key = strings.Join(append([]string{key}, args[1:]...), " ") key = strings.TrimSpace(key) } From e1c766610a7e9497a415959a29133d58aefaf134 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 19:49:10 +0100 Subject: [PATCH 010/148] fix(ui): rename copy variable to avoid shadowing builtin Co-Authored-By: Claude Opus 4.6 --- pkg/ui/pages/repo/repo.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/ui/pages/repo/repo.go b/pkg/ui/pages/repo/repo.go index 6d52d5511..369429558 100644 --- a/pkg/ui/pages/repo/repo.go +++ b/pkg/ui/pages/repo/repo.go @@ -115,11 +115,11 @@ func (r *Repo) commonHelp() []key.Binding { back.SetHelp("esc", "back to menu") tab := r.common.KeyMap.Section tab.SetHelp("tab", "switch tab") - copy := r.common.KeyMap.Copy - copy.SetHelp("c", "copy clone cmd") + copyKey := r.common.KeyMap.Copy + copyKey.SetHelp("c", "copy clone cmd") b = append(b, back) b = append(b, tab) - b = append(b, copy) + b = append(b, copyKey) return b } From 0a34735302a1e73dbde9c51c5def6a15a2bba3cb Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 19:49:56 +0100 Subject: [PATCH 011/148] fix(ui): allow token-auth users in TUI when AllowKeyless is false The TUI repo selection guard checked only for public key presence, blocking token-authenticated users when AllowKeyless was disabled. Now also checks for a context user (set by token auth middleware). Co-Authored-By: Claude Opus 4.6 --- pkg/ui/pages/selection/selection.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/ui/pages/selection/selection.go b/pkg/ui/pages/selection/selection.go index e66a8d552..ea01daf88 100644 --- a/pkg/ui/pages/selection/selection.go +++ b/pkg/ui/pages/selection/selection.go @@ -192,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 } From eb6e99ee59b0bb328a9896d7dbf9fee6e1de42d3 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Fri, 27 Mar 2026 20:27:11 +0100 Subject: [PATCH 012/148] refactor(ssh): remove dead code and unused parameters - Remove AccessLevelByPublicKey (zero callers after migration to AccessLevelForUser) - Remove unused *backend.Backend parameter from currentUser() - Clean up unused 'be' variables in info.go and pubkey.go Co-Authored-By: Claude Opus 4.6 --- pkg/backend/user.go | 25 ------------------------- pkg/ssh/cmd/cmd.go | 2 +- pkg/ssh/cmd/info.go | 4 +--- pkg/ssh/cmd/pubkey.go | 7 +++---- pkg/ssh/cmd/set_username.go | 2 +- 5 files changed, 6 insertions(+), 34 deletions(-) diff --git a/pkg/backend/user.go b/pkg/backend/user.go index 75335fc4e..297aab203 100644 --- a/pkg/backend/user.go +++ b/pkg/backend/user.go @@ -23,31 +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 { - if pk != nil { - 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()) - } - } - - // Fall back to the context user (e.g., authenticated via access token). - if ctxUser := proto.UserFromContext(ctx); ctxUser != nil { - return d.AccessLevel(ctx, repo, ctxUser.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 { diff --git a/pkg/ssh/cmd/cmd.go b/pkg/ssh/cmd/cmd.go index 25cd21ada..d82d4ebb9 100644 --- a/pkg/ssh/cmd/cmd.go +++ b/pkg/ssh/cmd/cmd.go @@ -113,7 +113,7 @@ func CommandName(args []string) string { // 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, _ *backend.Backend) (proto.User, error) { +func currentUser(ctx context.Context) (proto.User, error) { user := proto.UserFromContext(ctx) if user != nil { return user, nil diff --git a/pkg/ssh/cmd/info.go b/pkg/ssh/cmd/info.go index af52a4f88..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,8 +13,7 @@ func InfoCommand() *cobra.Command { Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() - be := backend.FromContext(ctx) - user, err := currentUser(ctx, be) + user, err := currentUser(ctx) if err != nil { return err } diff --git a/pkg/ssh/cmd/pubkey.go b/pkg/ssh/cmd/pubkey.go index 8acc0a811..d3273ac6e 100644 --- a/pkg/ssh/cmd/pubkey.go +++ b/pkg/ssh/cmd/pubkey.go @@ -23,7 +23,7 @@ func PubkeyCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) - user, err := currentUser(ctx, be) + user, err := currentUser(ctx) if err != nil { return err } @@ -44,7 +44,7 @@ func PubkeyCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) - user, err := currentUser(ctx, be) + user, err := currentUser(ctx) if err != nil { return err } @@ -65,8 +65,7 @@ func PubkeyCommand() *cobra.Command { Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() - be := backend.FromContext(ctx) - user, err := currentUser(ctx, be) + user, err := currentUser(ctx) if err != nil { return err } diff --git a/pkg/ssh/cmd/set_username.go b/pkg/ssh/cmd/set_username.go index c7ad1aa05..6a46cb17a 100644 --- a/pkg/ssh/cmd/set_username.go +++ b/pkg/ssh/cmd/set_username.go @@ -14,7 +14,7 @@ func SetUsernameCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() be := backend.FromContext(ctx) - user, err := currentUser(ctx, be) + user, err := currentUser(ctx) if err != nil { return err } From f98d1cc7bc9bc4559e42b39e23f3de57067f1324 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 06:17:15 +0100 Subject: [PATCH 013/148] fix(ssh): persist permissions in initializePermissions initializePermissions created a new permissions object when ctx.Permissions() returned nil but never called ctx.SetValue to persist it back, leaving a potential nil-pointer risk for subsequent callers of ctx.Permissions(). Co-Authored-By: Claude Opus 4.6 --- pkg/ssh/ssh.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/ssh/ssh.go b/pkg/ssh/ssh.go index 148beb841..0f9611c94 100644 --- a/pkg/ssh/ssh.go +++ b/pkg/ssh/ssh.go @@ -161,6 +161,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. From f02d5fc2263303a1f0627f71ffbc345860a14ba2 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 07:11:18 +0100 Subject: [PATCH 014/148] fix(ssh): store user ID instead of username in token auth extensions Eliminates a TOCTOU race where a username rename between KeyboardInteractiveHandler and AuthenticationMiddleware could resolve to the wrong user. Now stores the user ID and resolves via UserByID in the middleware. Also tightens test assertion from 'ssh-' to 'ssh-ed25519 '. Co-Authored-By: Claude Opus 4.6 --- pkg/ssh/middleware.go | 19 ++++++++++++------- pkg/ssh/ssh.go | 8 ++++---- testscript/testdata/token-ssh-auth.txtar | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/pkg/ssh/middleware.go b/pkg/ssh/middleware.go index 1c48c5d6a..e5a16b8f1 100644 --- a/pkg/ssh/middleware.go +++ b/pkg/ssh/middleware.go @@ -56,11 +56,11 @@ func AuthenticationMiddleware(sh ssh.Handler) ssh.Handler { } // Check if this session was authenticated via access token. - tokenUser := perms.Extensions[tokenUserExtKey] + tokenUserID := perms.Extensions[tokenUserExtKey] ac := be.AllowKeyless(ctx) - publicKeyCounter.WithLabelValues(strconv.FormatBool(ac || pk != nil || tokenUser != "")).Inc() - if !ac && pk == nil && tokenUser == "" { + publicKeyCounter.WithLabelValues(strconv.FormatBool(ac || pk != nil || tokenUserID != "")).Inc() + if !ac && pk == nil && tokenUserID == "" { wish.Fatalln(s, ErrPermissionDenied) return } @@ -69,11 +69,16 @@ func AuthenticationMiddleware(sh ssh.Handler) ssh.Handler { var user proto.User if pk != nil { user, _ = be.UserByPublicKey(ctx, pk) - } else if tokenUser != "" { - var err error - user, err = be.User(ctx, tokenUser) + } else if tokenUserID != "" { + uid, err := strconv.ParseInt(tokenUserID, 10, 64) if err != nil { - log.FromContext(ctx).Warn("token-authenticated user not found", "username", tokenUser, "err", err) + log.FromContext(ctx).Warn("invalid token user ID", "id", tokenUserID, "err", err) + wish.Fatalln(s, ErrPermissionDenied) + return + } + user, err = be.UserByID(ctx, uid) + if err != nil { + log.FromContext(ctx).Warn("token-authenticated user not found", "id", tokenUserID, "err", err) wish.Fatalln(s, ErrPermissionDenied) return } diff --git a/pkg/ssh/ssh.go b/pkg/ssh/ssh.go index 0f9611c94..7f261cca2 100644 --- a/pkg/ssh/ssh.go +++ b/pkg/ssh/ssh.go @@ -24,8 +24,8 @@ import ( ) // tokenUserExtKey is the permissions extension key used to pass the -// authenticated username from KeyboardInteractiveHandler to AuthenticationMiddleware. -const tokenUserExtKey = "token-user" +// authenticated user ID from KeyboardInteractiveHandler to AuthenticationMiddleware. +const tokenUserExtKey = "token-user-id" var ( publicKeyCounter = promauto.NewCounterVec(prometheus.CounterOpts{ @@ -197,9 +197,9 @@ func (s *SSHServer) KeyboardInteractiveHandler(ctx ssh.Context, challenge gossh. token := answers[0] user, tokenErr := s.be.UserByAccessToken(ctx, token) if tokenErr == nil && user != nil { - // Valid token: store user identity and allow access. + // Valid token: store user ID and allow access. perms.Extensions["pubkey-fp"] = "" - perms.Extensions[tokenUserExtKey] = user.Username() + perms.Extensions[tokenUserExtKey] = strconv.FormatInt(user.ID(), 10) ctx.SetValue(ssh.ContextKeyPermissions, perms) keyboardInteractiveCounter.WithLabelValues("true").Inc() s.logger.Info("keyboard-interactive token auth succeeded", "username", user.Username()) diff --git a/testscript/testdata/token-ssh-auth.txtar b/testscript/testdata/token-ssh-auth.txtar index ab23fb07c..71996cddc 100644 --- a/testscript/testdata/token-ssh-auth.txtar +++ b/testscript/testdata/token-ssh-auth.txtar @@ -51,7 +51,7 @@ stdout 'Username: user1' # TEST 7: pubkey list works with token auth tokensoft $TOKEN pubkey list -stdout 'ssh-' +stdout 'ssh-ed25519 ' # TEST 8: token list works with token auth (uses UserFromContext, already correct) tokensoft $TOKEN token list From c83651251da2222c5c87c0f36e3071ab85dd72fa Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 07:12:12 +0100 Subject: [PATCH 015/148] test(ssh): tighten assertions and add extra-args rejection test - Replace weak 'stderr .' with specific 'stderr no key found' - Add TEST 6: extra args without -k flag produce clear error Co-Authored-By: Claude Opus 4.6 --- testscript/testdata/user-create-key.txtar | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/testscript/testdata/user-create-key.txtar b/testscript/testdata/user-create-key.txtar index 12cf59492..f08777289 100644 --- a/testscript/testdata/user-create-key.txtar +++ b/testscript/testdata/user-create-key.txtar @@ -32,7 +32,11 @@ stdout 'Username: charlie' # TEST 5: invalid key is rejected ! soft user create dave -k 'not-a-valid-key' -stderr '.' +stderr 'no key found' + +# TEST 6: extra args without -k flag are rejected +! soft user create eve extraarg +stderr 'accepts 1 arg' # stop the server [windows] stopserver From c9e2c5ddc2e981acf27fd5fe6a9c900de4896122 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 07:33:05 +0100 Subject: [PATCH 016/148] fix(web): return io.ErrShortWrite and handle partial Read in ReadFrom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: when Write consumed fewer bytes than Read produced, the code returned n, err where err was nil — silently hiding data loss. Also, a Read that returns n>0 alongside io.EOF (valid per io.Reader contract) had its bytes dropped. Fix: restructure the loop to process any bytes returned before checking the read error, and return io.ErrShortWrite on a partial write. Closes charmbracelet/soft-serve#616 --- pkg/web/git.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/pkg/web/git.go b/pkg/web/git.go index 6723ac6f0..c570b8664 100644 --- a/pkg/web/git.go +++ b/pkg/web/git.go @@ -464,21 +464,26 @@ func (f *flushResponseWriter) ReadFrom(r io.Reader) (int64, error) { var n int64 p := make([]byte, 1024) for { - nRead, err := r.Read(p) - if err == io.EOF { - break - } - nWrite, err := f.ResponseWriter.Write(p[:nRead]) - if err != nil { - return n, err + nRead, readErr := r.Read(p) + if nRead > 0 { + nWrite, err := f.ResponseWriter.Write(p[:nRead]) + if err != nil { + return n, err + } + if nRead != nWrite { + return n, io.ErrShortWrite + } + n += int64(nRead) + // ResponseWriter must support http.Flusher to handle buffered output. + if err := flusher.Flush(); err != nil { + return n, fmt.Errorf("%w: error while flush", err) + } } - if nRead != nWrite { - return n, err + if readErr == io.EOF { + break } - n += int64(nRead) - // ResponseWriter must support http.Flusher to handle buffered output. - if err := flusher.Flush(); err != nil { - return n, fmt.Errorf("%w: error while flush", err) + if readErr != nil { + return n, readErr } } From 5c430e615e69367c3d1b53b2c140698c0e9305f3 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 07:46:51 +0100 Subject: [PATCH 017/148] fix(server): pre-bind listeners so port permission errors surface immediately Root cause: Start() launched all servers in errgroup goroutines. When one server failed to bind (e.g. EACCES on a privileged port), errg.Wait() had to wait for all other goroutines -- but those goroutines blocked in ListenAndServe() forever, so the error was never returned to the caller. Fix: bind all TCP listeners before launching goroutines. A bind failure is returned immediately from Start() before any server accepts connections. Each server gains a Serve(net.Listener) method. HTTPServer.Serve wraps the listener with TLS when configured. Closes charmbracelet/soft-serve#645 --- cmd/soft/serve/server.go | 47 +++++++++++++++++++++++++++++++++------- pkg/stats/stats.go | 6 +++++ pkg/web/http.go | 10 +++++++++ 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/cmd/soft/serve/server.go b/cmd/soft/serve/server.go index fda090050..f0ba3c533 100644 --- a/cmd/soft/serve/server.go +++ b/cmd/soft/serve/server.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "errors" "fmt" + "net" "net/http" "charm.land/log/v2" @@ -113,46 +114,76 @@ func (s *Server) ReloadCertificates() error { // Start starts the SSH server. func (s *Server) Start() error { + // Pre-bind all listeners before launching goroutines so that port + // permission errors (e.g. EACCES on privileged ports) are returned + // immediately instead of being swallowed by a blocked errgroup.Wait(). + var sshListener, gitListener, httpListener, statsListener net.Listener + var err error + + if s.Config.SSH.Enabled { + sshListener, err = net.Listen("tcp", s.Config.SSH.ListenAddr) + if err != nil { + return fmt.Errorf("listen ssh %s: %w", s.Config.SSH.ListenAddr, err) + } + } + + if s.Config.Git.Enabled { + gitListener, err = net.Listen("tcp", s.Config.Git.ListenAddr) + if err != nil { + return fmt.Errorf("listen git daemon %s: %w", s.Config.Git.ListenAddr, err) + } + } + + if s.Config.HTTP.Enabled { + httpListener, err = net.Listen("tcp", s.Config.HTTP.ListenAddr) + if err != nil { + return fmt.Errorf("listen http %s: %w", s.Config.HTTP.ListenAddr, err) + } + } + + if s.Config.Stats.Enabled { + statsListener, err = net.Listen("tcp", s.Config.Stats.ListenAddr) + if err != nil { + return fmt.Errorf("listen stats %s: %w", s.Config.Stats.ListenAddr, err) + } + } + errg, _ := errgroup.WithContext(s.ctx) - // 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(sshListener); !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(gitListener); !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(httpListener); !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(statsListener); !errors.Is(err, http.ErrServerClosed) { return err } return nil diff --git a/pkg/stats/stats.go b/pkg/stats/stats.go index 0ee72963e..fd68f4e21 100644 --- a/pkg/stats/stats.go +++ b/pkg/stats/stats.go @@ -2,6 +2,7 @@ package stats import ( "context" + "net" "net/http" "time" @@ -40,6 +41,11 @@ func (s *StatsServer) ListenAndServe() error { return s.server.ListenAndServe() } +// Serve starts the StatsServer on the given listener. +func (s *StatsServer) Serve(l net.Listener) error { + return s.server.Serve(l) +} + // Shutdown gracefully shuts down the StatsServer. func (s *StatsServer) Shutdown(ctx context.Context) error { return s.server.Shutdown(ctx) diff --git a/pkg/web/http.go b/pkg/web/http.go index 531d02bfd..c654f942f 100644 --- a/pkg/web/http.go +++ b/pkg/web/http.go @@ -3,6 +3,7 @@ package web import ( "context" "crypto/tls" + "net" "net/http" "time" @@ -56,6 +57,15 @@ func (s *HTTPServer) ListenAndServe() error { return s.Server.ListenAndServe() } +// Serve starts the HTTP server on the given listener. +// If TLS is configured the listener is wrapped before accepting connections. +func (s *HTTPServer) Serve(l net.Listener) error { + if s.Server.TLSConfig != nil { + l = tls.NewListener(l, s.Server.TLSConfig) + } + return s.Server.Serve(l) +} + // Shutdown gracefully shuts down the HTTP server. func (s *HTTPServer) Shutdown(ctx context.Context) error { return s.Server.Shutdown(ctx) From fdc7f1edf36853e04b5e9174d88b52dd62b2d0f2 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 07:58:15 +0100 Subject: [PATCH 018/148] fix(ui): use path instead of filepath for git tree paths on Windows filepath.Join and filepath.Dir use backslash on Windows. Git tree paths always use forward slashes, so TreePath lookups failed after the first directory level -- the path was built with backslashes but git expected forward slashes, causing navigation to silently revert to the parent directory on each Enter press. Replace filepath.Join/Dir with path.Join/Dir (the slash-only standard library package) in the TUI Files and Readme components. Closes charmbracelet/soft-serve#681 --- pkg/ui/pages/repo/files.go | 12 ++++++------ pkg/ui/pages/repo/readme.go | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/ui/pages/repo/files.go b/pkg/ui/pages/repo/files.go index 4c51e3561..77926e2e9 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" @@ -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/readme.go b/pkg/ui/pages/repo/readme.go index e7f1b9087..cc650d829 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" @@ -147,7 +147,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 " " } From d439099e124f429a4bf60cf81531b09a5bffd1d6 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 08:12:35 +0100 Subject: [PATCH 019/148] feat(config): add anon_access and allow_keyless config fields Operators can now set initial anonymous access level and keyless mode via config file or environment variables instead of requiring a manual post-start SSH session with the settings command. Both settings are applied to the database on every startup when configured, making it possible to stand up a fully automated server: SOFT_SERVE_ANON_ACCESS=no-access SOFT_SERVE_ALLOW_KEYLESS=false Or in config.yaml: anon_access: read-only allow_keyless: true The fields are optional: omitting them preserves whatever is in the database (set via the settings command). Validation in Validate() rejects unknown access-level strings at startup. Closes charmbracelet/soft-serve#758 --- cmd/soft/serve/serve.go | 22 +++++++++++ pkg/config/config.go | 27 ++++++++++++++ pkg/config/file.go | 11 ++++++ .../testdata/config-initial-settings.txtar | 37 +++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 testscript/testdata/config-initial-settings.txtar 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/pkg/config/config.go b/pkg/config/config.go index 97a5dc43d..e94158294 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" @@ -171,6 +172,16 @@ 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"` + // DataPath is the path to the directory where Soft Serve will store its data. DataPath string `env:"DATA_PATH" yaml:"-"` } @@ -190,6 +201,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), @@ -443,6 +455,21 @@ func (c *Config) Validate() error { c.InitialAdminKeys = pks + 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(), + ) + } + } + 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..7e75d740e 100644 --- a/pkg/config/file.go +++ b/pkg/config/file.go @@ -147,6 +147,17 @@ jobs: # 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 `)) func newConfigFile(cfg *Config) string { diff --git a/testscript/testdata/config-initial-settings.txtar b/testscript/testdata/config-initial-settings.txtar new file mode 100644 index 000000000..df7fe4c74 --- /dev/null +++ b/testscript/testdata/config-initial-settings.txtar @@ -0,0 +1,37 @@ +# vi: set ft=conf + +# Test that anon_access and allow_keyless set via environment variables are +# applied to the database on startup, overriding the migration defaults +# (read-only-access / true). + +env SOFT_SERVE_ANON_ACCESS=no-access +env SOFT_SERVE_ALLOW_KEYLESS=false + +exec soft serve & +ensureserverrunning SSH_PORT + +# Admin can still connect +soft info + +# Settings reflect what was configured, not the migration defaults +soft settings allow-keyless +stdout 'false' + +soft settings anon-access +stdout 'no-access' + +# Repo created by admin is not accessible to unregistered users under no-access +soft repo create pub1 +! ugit clone ssh://localhost:$SSH_PORT/pub1 upub1 +stderr 'not authorized|unauthorized' + +# Unregistered user cannot create repos either +! usoft repo create blocked +stderr 'unauthorized' + +# Change anon-access to read-only via admin and verify access is restored +soft settings anon-access read-only +ugit clone ssh://localhost:$SSH_PORT/pub1 upub1 + +# stop the server +[windows] stopserver From f8b73a1476980d380751f5c10b708b94d263d357 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 08:25:51 +0100 Subject: [PATCH 020/148] fix(ui): stop loading spinner when readme tab opens on empty repo EmptyRepoMsg handler was missing `r.isLoading = false`, leaving the Readme tab in a perpetual loading state for empty repositories. Closes #522 --- pkg/ui/pages/repo/readme.go | 1 + testscript/testdata/readme-empty-repo.txtar | 28 +++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 testscript/testdata/readme-empty-repo.txtar diff --git a/pkg/ui/pages/repo/readme.go b/pkg/ui/pages/repo/readme.go index e7f1b9087..c00bb2cd6 100644 --- a/pkg/ui/pages/repo/readme.go +++ b/pkg/ui/pages/repo/readme.go @@ -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"), diff --git a/testscript/testdata/readme-empty-repo.txtar b/testscript/testdata/readme-empty-repo.txtar new file mode 100644 index 000000000..b716b4b96 --- /dev/null +++ b/testscript/testdata/readme-empty-repo.txtar @@ -0,0 +1,28 @@ +# vi: set ft=conf + +# Regression test for https://github.com/charmbracelet/soft-serve/issues/522 +# Verifies that an empty repository can be created and queried without errors. +# The TUI bug (infinite loading spinner on Readme tab for empty repos) is fixed +# by setting r.isLoading = false in the EmptyRepoMsg handler in readme.go. + +# start soft serve +exec soft serve & +# wait for SSH server to start +ensureserverrunning SSH_PORT + +# create an empty repo (no initial commit, no README) +soft repo create empty-repo -d 'empty repository for issue 522' +stderr 'Created repository empty-repo.*' +stdout ssh://localhost:$SSH_PORT/empty-repo.git + +# verify the repo exists and reports correct metadata +soft repo info empty-repo +stdout 'Repository: empty-repo' +stdout 'Description: empty repository for issue 522' + +# verify the repo directory exists on disk +exists $DATA_PATH/repos/empty-repo.git + +# stop the server +[windows] stopserver +[windows] ! stderr . From 843b6da466b323900690446ad48be697e152576f Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 08:26:53 +0100 Subject: [PATCH 021/148] fix(ui): pressing copy key no longer duplicates items in filtered list When the filter was active and `c` was pressed, the list's items were being reset incorrectly, causing duplicate entries to appear. Closes #510 --- pkg/ui/pages/selection/item.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/ui/pages/selection/item.go b/pkg/ui/pages/selection/item.go index b6b3a02fa..1269b1236 100644 --- a/pkg/ui/pages/selection/item.go +++ b/pkg/ui/pages/selection/item.go @@ -139,10 +139,7 @@ func (d *ItemDelegate) Update(msg tea.Msg, m *list.Model) tea.Cmd { switch { case key.Matches(msg, d.common.KeyMap.Copy): d.copiedIdx = idx - return tea.Batch( - tea.SetClipboard(item.Command()), - m.SetItem(idx, item), - ) + return tea.SetClipboard(item.Command()) } } return nil From bdced3a982c9790ffdd7d2b3ccad1d58395f8ad6 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 08:30:14 +0100 Subject: [PATCH 022/148] fix(git): treat signal-killed git processes as client disconnects When a client (e.g. Flux's Source controller) performs only a capability advertisement check and then closes the connection, git-upload-pack is killed by signal. This is normal behavior for git's smart HTTP/SSH protocol but was being logged as ERR. Signal-killed git processes (ExitCode == -1) are now treated as clean client disconnects and not logged as errors. Closes #560 --- pkg/git/service.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index a4066d07e..cf806246e 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -163,8 +163,18 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er return ErrInvalidRepo } else if err != nil { var exitErr *exec.ExitError - if errors.As(err, &exitErr) && len(exitErr.Stderr) > 0 { - return fmt.Errorf("%s: %s", exitErr, exitErr.Stderr) + if errors.As(err, &exitErr) { + // ExitCode == -1 means the process was killed by a signal. + // This happens when a client (e.g. Flux's Source controller) + // performs only a capability advertisement check and then closes + // the connection. Treat this as a clean client disconnect, not + // an error. + if exitErr.ExitCode() == -1 { + return nil + } + if len(exitErr.Stderr) > 0 { + return fmt.Errorf("%s: %s", exitErr, exitErr.Stderr) + } } return err From 327532339a5449584d822c43a7d18ecc012214e2 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 08:50:30 +0100 Subject: [PATCH 023/148] fix(test): use single-word repo name in txtar test --- pkg/ssh/cmd/repo.go | 10 ++++++++-- testscript/testdata/readme-empty-repo.txtar | 15 ++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/pkg/ssh/cmd/repo.go b/pkg/ssh/cmd/repo.go index fa3e72ff9..29023fd08 100644 --- a/pkg/ssh/cmd/repo.go +++ b/pkg/ssh/cmd/repo.go @@ -4,6 +4,7 @@ import ( "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" @@ -58,7 +59,8 @@ func RepoCommand() *cobra.Command { } head, err := r.HEAD() - if err != nil { + isEmpty := err == git.ErrReferenceNotExist + if err != nil && !isEmpty { return err } @@ -84,7 +86,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/testscript/testdata/readme-empty-repo.txtar b/testscript/testdata/readme-empty-repo.txtar index b716b4b96..7057b1df2 100644 --- a/testscript/testdata/readme-empty-repo.txtar +++ b/testscript/testdata/readme-empty-repo.txtar @@ -11,17 +11,18 @@ exec soft serve & ensureserverrunning SSH_PORT # create an empty repo (no initial commit, no README) -soft repo create empty-repo -d 'empty repository for issue 522' -stderr 'Created repository empty-repo.*' -stdout ssh://localhost:$SSH_PORT/empty-repo.git +soft repo create emptyrepo -d 'emptyrepo-issue-522' +stderr 'Created repository emptyrepo.*' +stdout ssh://localhost:$SSH_PORT/emptyrepo.git # verify the repo exists and reports correct metadata -soft repo info empty-repo -stdout 'Repository: empty-repo' -stdout 'Description: empty repository for issue 522' +soft repo info emptyrepo +stdout 'Repository: emptyrepo' +stdout 'Description: emptyrepo-issue-522' +stdout 'Default Branch: \(empty repository\)' # verify the repo directory exists on disk -exists $DATA_PATH/repos/empty-repo.git +exists $DATA_PATH/repos/emptyrepo.git # stop the server [windows] stopserver From 85d0b64c6e90689acea52cbd1ddb158bf1ab533f Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 09:21:03 +0100 Subject: [PATCH 024/148] fix(backend): find README in docs/, .github/, .gitlab/ subdirectories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When no README exists at the repository root, soft-serve now falls back to checking docs/, .github/, and .gitlab/ subdirectories — matching GitHub/GitLab conventions. Root-level READMEs continue to take precedence over subdirectory ones. The Readme function previously stopped iterating on any non-ErrFileNotFound error; TreePath returns ErrRevisionNotExist when a subdirectory is absent, so that error is now also treated as "keep looking". Closes charmbracelet/soft-serve#634 Co-Authored-By: Claude Sonnet 4.6 --- pkg/backend/utils.go | 25 +++++- pkg/backend/utils_test.go | 166 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 2 deletions(-) create mode 100644 pkg/backend/utils_test.go diff --git a/pkg/backend/utils.go b/pkg/backend/utils.go index be1b0b4a7..930f0dc9f 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,28 @@ 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]*", + "docs/[rR][eE][aA][dD][mM][eE]*", + ".github/[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. 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) + 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) { + return + } + } return } diff --git a/pkg/backend/utils_test.go b/pkg/backend/utils_test.go new file mode 100644 index 000000000..a8da52441 --- /dev/null +++ b/pkg/backend/utils_test.go @@ -0,0 +1,166 @@ +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.Errorf("expected error when no readme exists, got content=%q path=%q", content, path) + } +} From 954ff5001b271d535ccfdbe5755c552393b80649 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 10:06:17 +0100 Subject: [PATCH 025/148] fix(ssh): use errors.Is for ErrReferenceNotExist sentinel comparison Co-Authored-By: Claude Sonnet 4.6 --- pkg/ssh/cmd/repo.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/ssh/cmd/repo.go b/pkg/ssh/cmd/repo.go index 29023fd08..e28fc8d80 100644 --- a/pkg/ssh/cmd/repo.go +++ b/pkg/ssh/cmd/repo.go @@ -1,6 +1,7 @@ package cmd import ( + "errors" "fmt" "strings" @@ -59,7 +60,7 @@ func RepoCommand() *cobra.Command { } head, err := r.HEAD() - isEmpty := err == git.ErrReferenceNotExist + isEmpty := errors.Is(err, git.ErrReferenceNotExist) if err != nil && !isEmpty { return err } From 8d328b32300d9217df9bc158cac5c1d90b72b967 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 10:09:09 +0100 Subject: [PATCH 026/148] fix(git): only suppress signal-killed errors when context is cancelled Co-Authored-By: Claude Sonnet 4.6 --- pkg/git/service.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index cf806246e..de20e970c 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -164,16 +164,13 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er } else if err != nil { var exitErr *exec.ExitError if errors.As(err, &exitErr) { - // ExitCode == -1 means the process was killed by a signal. - // This happens when a client (e.g. Flux's Source controller) - // performs only a capability advertisement check and then closes - // the connection. Treat this as a clean client disconnect, not - // an error. - if exitErr.ExitCode() == -1 { + if exitErr.ExitCode() == -1 && ctx.Err() != nil { + // Process was killed because context was cancelled (client disconnected). + // This is normal for capability-advertisement-only clients (e.g. git ls-remote). return nil } if len(exitErr.Stderr) > 0 { - return fmt.Errorf("%s: %s", exitErr, exitErr.Stderr) + return fmt.Errorf("%s: %w", exitErr.Stderr, err) } } From 6df9001162385f50581b2fb8d6bb61155b84e2bd Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 10:12:16 +0100 Subject: [PATCH 027/148] fix(backend): return nil error from Readme when no README found Return ("", "", nil) instead of the last iteration's error when no README exists in any pattern location, so callers detect missing READMEs by checking path == "" rather than err != nil. Update selection.go caller to guard on path == "" in addition to err. Add bare-repo test cases to cover production repo format. Co-Authored-By: Claude Sonnet 4.6 --- pkg/backend/utils.go | 6 +++- pkg/backend/utils_test.go | 51 +++++++++++++++++++++++++++-- pkg/ui/pages/selection/selection.go | 2 +- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/pkg/backend/utils.go b/pkg/backend/utils.go index 930f0dc9f..0c7284c03 100644 --- a/pkg/backend/utils.go +++ b/pkg/backend/utils.go @@ -30,6 +30,8 @@ var readmePatterns = []string{ // 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) { for _, pattern := range readmePatterns { readme, path, err = LatestFile(r, ref, pattern) @@ -40,5 +42,7 @@ func Readme(r proto.Repository, ref *git.Reference) (readme string, path string, return } } - 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 index a8da52441..bf05f99ff 100644 --- a/pkg/backend/utils_test.go +++ b/pkg/backend/utils_test.go @@ -160,7 +160,54 @@ func TestReadme_NotFound(t *testing.T) { }) repo := &stubRepo{path: dir} content, path, err := Readme(repo, nil) - if err == nil { - t.Errorf("expected error when no readme exists, got content=%q path=%q", content, path) + 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/ui/pages/selection/selection.go b/pkg/ui/pages/selection/selection.go index 270cf2bb7..18900d725 100644 --- a/pkg/ui/pages/selection/selection.go +++ b/pkg/ui/pages/selection/selection.go @@ -203,7 +203,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 } From bcdf3352078b013c37c8219e39b5ca8699f5e794 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 10:26:05 +0100 Subject: [PATCH 028/148] fix(ui): persist copied feedback for 1.5s using timer reset message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous approach reset d.copiedIdx = -1 inside Render(), so the "(copied to clipboard)" indicator only appeared for one frame — invisible in practice because SetItem triggers an immediate filterItems re-render. Introduce copiedResetMsg and a 1500ms timer command so the feedback stays visible across re-renders. Also adds m.SetItem(m.GlobalIndex(), item) to trigger the re-render that makes the indicator appear in the first place. Co-Authored-By: Claude Sonnet 4.6 --- pkg/ui/pages/selection/item.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/ui/pages/selection/item.go b/pkg/ui/pages/selection/item.go index 1269b1236..9fc46e623 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{} + // Items is a list of Item. type Items []Item @@ -139,8 +142,17 @@ func (d *ItemDelegate) Update(msg tea.Msg, m *list.Model) tea.Cmd { switch { case key.Matches(msg, d.common.KeyMap.Copy): d.copiedIdx = idx - return tea.SetClipboard(item.Command()) + return tea.Batch( + tea.SetClipboard(item.Command()), + m.SetItem(m.GlobalIndex(), item), + func() tea.Msg { + time.Sleep(1500 * time.Millisecond) + return copiedResetMsg{} + }, + ) } + case copiedResetMsg: + d.copiedIdx = -1 } return nil } @@ -207,7 +219,6 @@ func (d *ItemDelegate) Render(w io.Writer, m list.Model, index int, listItem lis if d.copiedIdx == index { cmd = "(copied to clipboard)" cmdStyler = styles.Desc.Render - d.copiedIdx = -1 } cmd = common.TruncateString(cmd, m.Width()-styles.Base.GetHorizontalFrameSize()) s.WriteString(cmdStyler(cmd)) From 130eab5bb02c3e282e63e818189ed45e724c44c2 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 10:30:59 +0100 Subject: [PATCH 029/148] fix(git): narrow signal-kill suppression to context.Canceled only context.DeadlineExceeded (server timeout) should still propagate as an error. Only suppress signal-killed processes on context.Canceled which indicates a clean client disconnect. --- pkg/git/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index de20e970c..b115be2f0 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -164,7 +164,7 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er } else if err != nil { var exitErr *exec.ExitError if errors.As(err, &exitErr) { - if exitErr.ExitCode() == -1 && ctx.Err() != nil { + if exitErr.ExitCode() == -1 && errors.Is(ctx.Err(), context.Canceled) { // Process was killed because context was cancelled (client disconnected). // This is normal for capability-advertisement-only clients (e.g. git ls-remote). return nil From 61c9e103127ca1246cb7e4393869a991ff502afd Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 10:31:01 +0100 Subject: [PATCH 030/148] fix(ui): use generation counter to prevent stale copied-indicator reset A rapid double-press of c would cause the first goroutine's copiedResetMsg to clear the indicator while the second copy was still showing. The generation counter ensures only the most recent reset message takes effect. --- pkg/ui/pages/selection/item.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/ui/pages/selection/item.go b/pkg/ui/pages/selection/item.go index 9fc46e623..d834ea0b6 100644 --- a/pkg/ui/pages/selection/item.go +++ b/pkg/ui/pages/selection/item.go @@ -19,7 +19,7 @@ import ( var _ sort.Interface = Items{} // copiedResetMsg is sent after the copy feedback timer expires. -type copiedResetMsg struct{} +type copiedResetMsg struct{ gen int } // Items is a list of Item. type Items []Item @@ -104,6 +104,7 @@ type ItemDelegate struct { common *common.Common activePane *pane copiedIdx int + copiedGen int } // NewItemDelegate creates a new ItemDelegate. @@ -142,17 +143,21 @@ func (d *ItemDelegate) Update(msg tea.Msg, m *list.Model) tea.Cmd { switch { case key.Matches(msg, d.common.KeyMap.Copy): d.copiedIdx = idx + d.copiedGen++ + gen := d.copiedGen return tea.Batch( tea.SetClipboard(item.Command()), m.SetItem(m.GlobalIndex(), item), func() tea.Msg { time.Sleep(1500 * time.Millisecond) - return copiedResetMsg{} + return copiedResetMsg{gen: gen} }, ) } case copiedResetMsg: - d.copiedIdx = -1 + if msg.gen == d.copiedGen { + d.copiedIdx = -1 + } } return nil } From c5368017e712623e77180900de8209c8d53e769a Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 10:43:32 +0100 Subject: [PATCH 031/148] fix(ui): store GlobalIndex in copiedIdx so Render highlights correct item idx from ItemDelegate.Update is the filtered position; Render receives the global item index. Using m.GlobalIndex() ensures they match even when a search filter is active. --- pkg/ui/pages/selection/item.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/ui/pages/selection/item.go b/pkg/ui/pages/selection/item.go index d834ea0b6..a79e411f5 100644 --- a/pkg/ui/pages/selection/item.go +++ b/pkg/ui/pages/selection/item.go @@ -133,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 @@ -142,7 +141,7 @@ 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.copiedIdx = m.GlobalIndex() d.copiedGen++ gen := d.copiedGen return tea.Batch( From 3a7f8a298d9e2e91ec71941e2fb77361a1a66d2d Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 10:56:00 +0100 Subject: [PATCH 032/148] fix(ui): track copied item by identity not index to fix pagination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit m.GlobalIndex() diverges from the page-local index Render receives when the list is paginated. Store the copied Item value directly and compare by identity in Render — unambiguous regardless of pagination or filtering. --- pkg/ui/pages/selection/item.go | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/pkg/ui/pages/selection/item.go b/pkg/ui/pages/selection/item.go index a79e411f5..64e842e29 100644 --- a/pkg/ui/pages/selection/item.go +++ b/pkg/ui/pages/selection/item.go @@ -19,7 +19,7 @@ import ( var _ sort.Interface = Items{} // copiedResetMsg is sent after the copy feedback timer expires. -type copiedResetMsg struct{ gen int } +type copiedResetMsg struct{} // Items is a list of Item. type Items []Item @@ -101,10 +101,10 @@ func (i Item) Command() string { // ItemDelegate is the delegate for the item. type ItemDelegate struct { - common *common.Common - activePane *pane - copiedIdx int - copiedGen int + common *common.Common + activePane *pane + copiedItem Item + hasCopied bool } // NewItemDelegate creates a new ItemDelegate. @@ -112,7 +112,6 @@ func NewItemDelegate(common *common.Common, activePane *pane) *ItemDelegate { return &ItemDelegate{ common: common, activePane: activePane, - copiedIdx: -1, } } @@ -141,22 +140,20 @@ 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 = m.GlobalIndex() - d.copiedGen++ - gen := d.copiedGen + d.copiedItem = item + d.hasCopied = true return tea.Batch( tea.SetClipboard(item.Command()), m.SetItem(m.GlobalIndex(), item), func() tea.Msg { time.Sleep(1500 * time.Millisecond) - return copiedResetMsg{gen: gen} + return copiedResetMsg{} }, ) } case copiedResetMsg: - if msg.gen == d.copiedGen { - d.copiedIdx = -1 - } + d.hasCopied = false + d.copiedItem = Item{} } return nil } @@ -220,7 +217,7 @@ 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 } From fd90b7ed417970f9d4764c70b714c2b779cf622b Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:01:26 +0100 Subject: [PATCH 033/148] fix(ui): restore generation counter to prevent premature copied-state reset Removing the generation counter meant rapid double-copy would clear the second copy's indicator when the first timer fired. The gen counter ensures only the most recent reset message takes effect. Co-Authored-By: Claude Sonnet 4.6 --- pkg/ui/pages/selection/item.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/ui/pages/selection/item.go b/pkg/ui/pages/selection/item.go index 64e842e29..1782128ce 100644 --- a/pkg/ui/pages/selection/item.go +++ b/pkg/ui/pages/selection/item.go @@ -19,7 +19,7 @@ import ( var _ sort.Interface = Items{} // copiedResetMsg is sent after the copy feedback timer expires. -type copiedResetMsg struct{} +type copiedResetMsg struct{ gen int } // Items is a list of Item. type Items []Item @@ -105,6 +105,7 @@ type ItemDelegate struct { activePane *pane copiedItem Item hasCopied bool + copiedGen int } // NewItemDelegate creates a new ItemDelegate. @@ -142,18 +143,22 @@ func (d *ItemDelegate) Update(msg tea.Msg, m *list.Model) tea.Cmd { case key.Matches(msg, d.common.KeyMap.Copy): d.copiedItem = item d.hasCopied = true + d.copiedGen++ + gen := d.copiedGen return tea.Batch( tea.SetClipboard(item.Command()), m.SetItem(m.GlobalIndex(), item), func() tea.Msg { time.Sleep(1500 * time.Millisecond) - return copiedResetMsg{} + return copiedResetMsg{gen: gen} }, ) } case copiedResetMsg: - d.hasCopied = false - d.copiedItem = Item{} + if msg.gen == d.copiedGen { + d.hasCopied = false + d.copiedItem = Item{} + } } return nil } From 2b23308ce624e0352430fc16acbb2176435a07b4 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:02:54 +0100 Subject: [PATCH 034/148] =?UTF-8?q?fix(git):=20restore=20error=20format=20?= =?UTF-8?q?order=20=E2=80=94=20wrap=20ExitError,=20append=20stderr=20as=20?= =?UTF-8?q?context?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- pkg/git/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index b115be2f0..1a2444cc6 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -170,7 +170,7 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er return nil } if len(exitErr.Stderr) > 0 { - return fmt.Errorf("%s: %w", exitErr.Stderr, err) + return fmt.Errorf("%w: %s", err, exitErr.Stderr) } } From 6809665a18c1a8ea2ed842a5457a303d1fa6155a Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:02:57 +0100 Subject: [PATCH 035/148] fix(backend): handle ErrReferenceNotExist in README search loop Empty repositories (no HEAD) caused LatestFile to return ErrReferenceNotExist, which was not in the loop's continue condition. This caused Readme() to return an error instead of ("","",nil). Co-Authored-By: Claude Sonnet 4.6 --- pkg/backend/utils.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/backend/utils.go b/pkg/backend/utils.go index 0c7284c03..1aef1dc93 100644 --- a/pkg/backend/utils.go +++ b/pkg/backend/utils.go @@ -38,7 +38,9 @@ func Readme(r proto.Repository, ref *git.Reference) (readme string, path string, if err == nil { return } - if !errors.Is(err, git.ErrFileNotFound) && !errors.Is(err, git.ErrRevisionNotExist) { + if !errors.Is(err, git.ErrFileNotFound) && + !errors.Is(err, git.ErrRevisionNotExist) && + !errors.Is(err, git.ErrReferenceNotExist) { return } } From 02d28da2f0463c39618c5226e8e1d7eefef2a184 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:07:04 +0100 Subject: [PATCH 036/148] fix(git): log suppressed signal-kill at debug level instead of silently dropping During server shutdown or client disconnect, context.Canceled is set and the killed git process returns nil. Log at debug so the suppression is visible when debugging without polluting normal operation logs. Co-Authored-By: Claude Sonnet 4.6 --- pkg/git/service.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index 1a2444cc6..e46c29afc 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -165,8 +165,11 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er var exitErr *exec.ExitError if errors.As(err, &exitErr) { if exitErr.ExitCode() == -1 && errors.Is(ctx.Err(), context.Canceled) { - // Process was killed because context was cancelled (client disconnected). - // This is normal for capability-advertisement-only clients (e.g. git ls-remote). + // Process was killed because context was cancelled — either a client + // that disconnected after capability advertisement (e.g. git ls-remote) + // or an in-flight operation killed during server shutdown. Both are + // expected and not worth surfacing as errors. + log.FromContext(ctx).Debug("git process killed on context cancellation", "service", svc.Name()) return nil } if len(exitErr.Stderr) > 0 { From d67403c2834be8c49eb0344024578d75945c84b8 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:13:50 +0100 Subject: [PATCH 037/148] =?UTF-8?q?fix(ui):=20remove=20stale=20SetItem=20c?= =?UTF-8?q?all=20=E2=80=94=20render=20now=20uses=20item=20identity=20not?= =?UTF-8?q?=20index?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- pkg/ui/pages/selection/item.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/ui/pages/selection/item.go b/pkg/ui/pages/selection/item.go index 1782128ce..991582c0e 100644 --- a/pkg/ui/pages/selection/item.go +++ b/pkg/ui/pages/selection/item.go @@ -147,7 +147,6 @@ func (d *ItemDelegate) Update(msg tea.Msg, m *list.Model) tea.Cmd { gen := d.copiedGen return tea.Batch( tea.SetClipboard(item.Command()), - m.SetItem(m.GlobalIndex(), item), func() tea.Msg { time.Sleep(1500 * time.Millisecond) return copiedResetMsg{gen: gen} From 0b5f3b75ea875dfda3e7a64de0fc3b7903a0101e Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:13:52 +0100 Subject: [PATCH 038/148] fix(backend): tighten README glob to avoid matching READMEFILE and similar Co-Authored-By: Claude Sonnet 4.6 --- pkg/backend/utils.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/backend/utils.go b/pkg/backend/utils.go index 1aef1dc93..03939c9e6 100644 --- a/pkg/backend/utils.go +++ b/pkg/backend/utils.go @@ -21,10 +21,14 @@ func LatestFile(r proto.Repository, ref *git.Reference, pattern string) (string, // 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]*", - "docs/[rR][eE][aA][dD][mM][eE]*", - ".github/[rR][eE][aA][dD][mM][eE]*", - ".gitlab/[rR][eE][aA][dD][mM][eE]*", + "[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. From 37193a6be9d107c26ff4c486e72457b20cae2d07 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:16:08 +0100 Subject: [PATCH 039/148] fix(git): verify and document WaitDelay/ExitError interaction on cancellation --- pkg/git/service.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/git/service.go b/pkg/git/service.go index e46c29afc..ad8f795f1 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -163,6 +163,11 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er return ErrInvalidRepo } else if err != nil { var exitErr *exec.ExitError + // 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 exitErr.ExitCode() == -1 && errors.Is(ctx.Err(), context.Canceled) { // Process was killed because context was cancelled — either a client @@ -175,6 +180,12 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er if len(exitErr.Stderr) > 0 { return fmt.Errorf("%w: %s", err, exitErr.Stderr) } + } else if errors.Is(ctx.Err(), context.Canceled) { + // Fallback: context was cancelled but the error is not an ExitError + // (e.g. WaitDelay produced a pure timeout error without an underlying + // ExitError). Suppress this as well — it is expected cleanup noise. + log.FromContext(ctx).Debug("git process cleanup on context cancellation", "service", svc.Name()) + return nil } return err From dbfe3ad5104d9985f6fabc52ef769241f71aef1d Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:20:19 +0100 Subject: [PATCH 040/148] fix(git): narrow WaitDelay fallback to exec.ErrWaitDelay specifically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous else-if checked only ctx.Err()==context.Canceled which could suppress genuine pipe/IO errors. Check exec.ErrWaitDelay explicitly — the only non-ExitError produced by WaitDelay expiry — before suppressing. --- pkg/git/service.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index ad8f795f1..f058fc479 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -180,11 +180,10 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er if len(exitErr.Stderr) > 0 { return fmt.Errorf("%w: %s", err, exitErr.Stderr) } - } else if errors.Is(ctx.Err(), context.Canceled) { - // Fallback: context was cancelled but the error is not an ExitError - // (e.g. WaitDelay produced a pure timeout error without an underlying - // ExitError). Suppress this as well — it is expected cleanup noise. - log.FromContext(ctx).Debug("git process cleanup on context cancellation", "service", svc.Name()) + } else if errors.Is(err, exec.ErrWaitDelay) && errors.Is(ctx.Err(), context.Canceled) { + // WaitDelay expired after context cancellation — the process was already + // killed; pipe goroutines did not drain within 30s. Not an error. + log.FromContext(ctx).Debug("git pipe drain timed out after cancellation", "service", svc.Name()) return nil } From 831ae91984c6940d31ff41cf06cb30216019c857 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:22:09 +0100 Subject: [PATCH 041/148] fix(git): use forward slashes in LatestFile path matching for Windows compat filepath.Join produces OS-native separators (backslash on Windows) but glob patterns and git tree paths always use forward slashes. Use path.Join (always forward slash) so subdirectory README patterns match on Windows. --- git/utils.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git/utils.go b/git/utils.go index b4ca50fc6..922f6568f 100644 --- a/git/utils.go +++ b/git/utils.go @@ -2,6 +2,7 @@ package git import ( "os" + "path" "path/filepath" "github.com/gobwas/glob" @@ -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 } From 79cbb016bf5633607d463b15131b2b9ac21223a1 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:25:21 +0100 Subject: [PATCH 042/148] fix(git): use path.Dir instead of filepath.Dir in LatestFile for Windows compat On Windows, filepath.Dir produces backslash-separated paths. Combined with the existing path.Join on line 31, this creates mixed separators that break glob matching. Switch to path.Dir to keep all virtual path operations consistently using forward slashes. --- git/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/utils.go b/git/utils.go index 922f6568f..5e1e72559 100644 --- a/git/utils.go +++ b/git/utils.go @@ -11,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 { From f3eb83ff96f92b58cf27f4183156b3ed4972b2ff Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:26:50 +0100 Subject: [PATCH 043/148] fix(git): check erro not err in stderr copy goroutine Pre-existing bug: the goroutine logging stderr copy failures checked the outer err variable (always nil at that point) instead of the local erro return value, silently swallowing all stderr pipe errors. --- pkg/git/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index f058fc479..f53c75703 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -147,7 +147,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 { log.Errorf("gitServiceHandler: failed to copy stderr: %v", erro) } }() From 4736251e3a1c04a5edb5ccf7aaa86e37c7adf301 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:31:44 +0100 Subject: [PATCH 044/148] fix(ui): propagate backend.Readme error instead of silently discarding Disk I/O and permission errors were swallowed, leaving users with a blank readme and no indication of failure. Return ErrorMsg on non-nil errors so the TUI surfaces them correctly. Co-Authored-By: Claude Sonnet 4.6 --- pkg/ui/pages/repo/readme.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/ui/pages/repo/readme.go b/pkg/ui/pages/repo/readme.go index c00bb2cd6..610085a64 100644 --- a/pkg/ui/pages/repo/readme.go +++ b/pkg/ui/pages/repo/readme.go @@ -165,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 From 5d8d087d92cb1aba2c889098233f026a6b7979d0 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:35:31 +0100 Subject: [PATCH 045/148] fix(git): make context-cancel suppression portable across Windows/Unix ExitCode()==-1 only signals a killed process on Unix; on Windows, TerminateProcess sets exit code 1. Gate only on ctx.Err()==Canceled which is true on both platforms when we killed the process. Co-Authored-By: Claude Sonnet 4.6 --- pkg/git/service.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index f53c75703..5631edc86 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -169,12 +169,13 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er // returns true. So the suppression path below is safe even when WaitDelay // wraps the ExitError via errors.Join. if errors.As(err, &exitErr) { - if exitErr.ExitCode() == -1 && errors.Is(ctx.Err(), context.Canceled) { - // Process was killed because context was cancelled — either a client - // that disconnected after capability advertisement (e.g. git ls-remote) - // or an in-flight operation killed during server shutdown. Both are - // expected and not worth surfacing as errors. - log.FromContext(ctx).Debug("git process killed on context cancellation", "service", svc.Name()) + 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 } if len(exitErr.Stderr) > 0 { From c99245ea7b410896a7ed0180c9dbe719d0809b85 Mon Sep 17 00:00:00 2001 From: Dan Castrillo Date: Sat, 28 Mar 2026 11:40:33 +0100 Subject: [PATCH 046/148] chore(git): remove dead os.ErrNotExist branch after cmd.Wait() cmd.Wait() never returns os.ErrNotExist; the ErrInvalidRepo translation at Wait-time was unreachable dead code copied from the cmd.Start() guard. Co-Authored-By: Claude Sonnet 4.6 --- pkg/git/service.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index 5631edc86..29445cd7e 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -159,9 +159,7 @@ 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 // Note: errors.As correctly unwraps through errors.Join, which Go 1.20+ // uses when cmd.WaitDelay fires (joining the ExitError with a timeout From 35defc7c092fec659b62549b54c1c8c917cbd1d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=80=E3=83=8B=E3=82=A8=E3=83=AB=E3=83=BB=E3=82=AB?= =?UTF-8?q?=E3=82=B9=E3=83=88=E3=83=AA=E3=83=AD?= <126793278+dvrd@users.noreply.github.com> Date: Sat, 28 Mar 2026 14:23:46 +0100 Subject: [PATCH 047/148] fix(ssh): user create -k correctly handles space-separated public keys (#16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ssh): user create correctly handles space-separated public keys SSH exec tokenizes commands on spaces, splitting "ssh-ed25519 AAAA..." into separate args. Reconstruct the full key from the -k flag value and any trailing positional args. Also handles the positional-only path (no -k flag) and rejects an empty -k "" value. Closes #750 Co-Authored-By: Claude Sonnet 4.6 * fix(ssh): replace hardcoded key-type allowlist with ParseAuthorizedKey probe The validKeyPrefixes slice (ssh-, ecdsa-, sk-) would silently reject future key types (xmss-, post-quantum, etc.). Replace the prefix check with a full ParseAuthorizedKey probe on the reconstructed key — the parser is the authoritative validator and handles all current and future key types without a maintained allowlist. Co-Authored-By: Claude Sonnet 4.6 * fix(ssh): show full reconstructed key in unexpected-argument error The error message for an unrecognised positional argument cited only args[1] (the key-type token) even when multiple extra tokens were passed. Show the full joined string so the user sees exactly what was rejected. Co-Authored-By: Claude Sonnet 4.6 * refactor(ssh): remove redundant ParseAuthorizedKey probe in user create The probe validated the same joined string that ParseAuthorizedKey at line 72 would parse immediately after, making it a redundant double parse with a misleading error frame ("unexpected argument") for what is really a parse failure. Remove the probe; the final parse provides an accurate error message. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- pkg/ssh/cmd/user.go | 39 +++++++++----- .../testdata/user-create-with-key.txtar | 53 +++++++++++++++++++ 2 files changed, 80 insertions(+), 12 deletions(-) create mode 100644 testscript/testdata/user-create-with-key.txtar diff --git a/pkg/ssh/cmd/user.go b/pkg/ssh/cmd/user.go index 0e7b03c7a..fd794f90a 100644 --- a/pkg/ssh/cmd/user.go +++ b/pkg/ssh/cmd/user.go @@ -23,8 +23,11 @@ func UserCommand() *cobra.Command { var admin bool var key string userCreateCommand := &cobra.Command{ - Use: "create USERNAME", - Short: "Create a new user", + Use: "create USERNAME", + Short: "Create a new user", + Long: `Create a new user. The public key can be provided via the -k/--key flag. +When connecting over SSH, the key is automatically reconstructed if the +SSH protocol splits it on spaces.`, Args: cobra.MinimumNArgs(1), PersistentPreRunE: checkIfAdmin, RunE: func(cmd *cobra.Command, args []string) error { @@ -32,19 +35,31 @@ func UserCommand() *cobra.Command { ctx := cmd.Context() be := backend.FromContext(ctx) username := args[0] - // When -k is passed over SSH, the key value may be split across - // the flag and trailing positional args (e.g. -k "ssh-ed25519 AAAA" - // becomes -k ssh-ed25519 AAAA as separate tokens). Merge any - // trailing args into the key to reassemble the full value. + // SSH exec tokenizes commands on spaces, so a public key like + // "ssh-ed25519 AAAA..." arrives as separate args. Reconstruct + // the full key by joining the flag value with any trailing args. + // Use a local variable to avoid mutating the closure-captured flag + // var, which would bleed state into subsequent invocations. + localKey := key + if cmd.Flags().Changed("key") && localKey == "" { + return fmt.Errorf("--key flag requires a non-empty public key value") + } if len(args) > 1 { - if key == "" { - return fmt.Errorf("accepts 1 arg(s), received %d", len(args)) + // SSH exec tokenizes the command on spaces, splitting "ssh-ed25519 AAAA..." + // into separate args. Reconstruct only when localKey is a partial key type + // (no space = just "ssh-ed25519") or when no -k flag was given. + if strings.Contains(localKey, " ") { + // localKey is already a complete key; extra positional args are unexpected + return fmt.Errorf("unexpected arguments: %v", args[1:]) + } + parts := args[1:] + if localKey != "" { + parts = append([]string{localKey}, parts...) } - key = strings.Join(append([]string{key}, args[1:]...), " ") - key = strings.TrimSpace(key) + localKey = strings.Join(parts, " ") } - if key != "" { - pk, _, err := sshutils.ParseAuthorizedKey(key) + if localKey != "" { + pk, _, err := sshutils.ParseAuthorizedKey(localKey) if err != nil { return err } diff --git a/testscript/testdata/user-create-with-key.txtar b/testscript/testdata/user-create-with-key.txtar new file mode 100644 index 000000000..bffca7cac --- /dev/null +++ b/testscript/testdata/user-create-with-key.txtar @@ -0,0 +1,53 @@ +# vi: set ft=conf + +# Regression test for https://github.com/charmbracelet/soft-serve/issues/750 +# SSH exec tokenizes commands on spaces, splitting "ssh-ed25519 AAAA..." +# into separate args. The -k flag should reconstruct the full key. + +# convert crlf to lf on windows +[windows] dos2unix user_info.txt + +# start soft serve +exec soft serve & +# wait for SSH server to start +ensureserverrunning SSH_PORT + +# Create a user passing the public key via -k flag. +# $USER1_AUTHORIZED_KEY expands to "ssh-ed25519 AAAA..." (two space-separated +# tokens). When sent over SSH exec and re-split by the server, the key type +# and key data become separate positional args — this is the bug from #750. +soft user create testuser -k $USER1_AUTHORIZED_KEY + +# Verify the user exists and has the key attached. +soft user info testuser +cmpenv stdout user_info.txt + +# Create a second user passing the public key as positional args (no -k flag). +# This exercises the SSH-exec tokenization reconstruction path: the client +# sends "user create testuser2 ssh-ed25519 AAAA..." and the server receives +# "testuser2", "ssh-ed25519", "AAAA..." as separate positional args. +soft user create testuser2 $USER1_AUTHORIZED_KEY + +# Verify the second user also has the key attached correctly. +soft user info testuser2 +cmpenv stdout user2_info.txt + +# Reject empty -k flag value. +! soft user create baduser -k '' +stderr 'non-empty' + +# stop the server +[windows] stopserver +[windows] ! stderr . + +-- user_info.txt -- +Username: testuser +Admin: false +Public keys: + $USER1_AUTHORIZED_KEY + +-- user2_info.txt -- +Username: testuser2 +Admin: false +Public keys: + $USER1_AUTHORIZED_KEY From f37f6e84f6b6c488182eae526f72b8b769e0ae54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=80=E3=83=8B=E3=82=A8=E3=83=AB=E3=83=BB=E3=82=AB?= =?UTF-8?q?=E3=82=B9=E3=83=88=E3=83=AA=E3=83=AD?= <126793278+dvrd@users.noreply.github.com> Date: Sat, 28 Mar 2026 14:25:39 +0100 Subject: [PATCH 048/148] fix(git): prevent zombie processes on context-cancelled git commands (#17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(git): prevent zombie processes when git commands are context-cancelled Without cmd.WaitDelay, a context-cancelled exec.CommandContext sends SIGKILL but the process entry lingers until Wait() is called. Setting WaitDelay ensures the OS process is reaped even if the stdin pipe goroutine is still running. Closes #797 * fix(git): clarify WaitDelay semantics — post-termination only Co-Authored-By: Claude Sonnet 4.6 * fix(git): document that ctx carries no deadline in gitServiceHandler SSH and git-daemon timeouts operate at the net.Conn layer via SetDeadline, not via context.WithDeadline. The context passed here is cancellation-only, so WaitDelay cannot interfere with legitimate large pushes or clones. * fix(git): suppress pipe-drain error logs on context cancellation When the git process is killed after context cancellation, the stdout, stderr and stdin io.Copy goroutines return broken-pipe errors. These were logged at Error level, producing spurious noise on every client disconnect. Guard each log with ctx.Err() == nil so errors are only reported for genuinely unexpected I/O failures. Co-Authored-By: Claude Sonnet 4.6 * fix(git): suppress ErrWaitDelay unconditionally when no ExitError present When git exits cleanly (code 0) but pipe goroutines stall for longer than WaitDelay (30s), cmd.Wait() returns exec.ErrWaitDelay alone with no ExitError. The previous guard required ctx.Err()==Canceled, so a slow-reading client on a non-cancelled context would receive a spurious error even though the git command succeeded. Remove the context check: this branch is only reached when there is no ExitError component, meaning git exited 0. ErrWaitDelay is an internal drain timeout, not a caller-visible error in that case. Co-Authored-By: Claude Sonnet 4.6 * fix(git): strip ErrWaitDelay noise from non-zero-exit errors When git exits non-zero and WaitDelay also fires, cmd.Wait returns errors.Join(exitErr, exec.ErrWaitDelay). The caller was receiving that joined error string, which includes "exec: WaitDelay expired" noise alongside the real exit-status error. Return exitErr directly (with stderr appended if present) so callers see a clean exit-status error, not an internal drain-timeout detail. Co-Authored-By: Claude Sonnet 4.6 * fix(git): downgrade stdin goroutine broken-pipe log to debug When git exits cleanly before the client finishes sending, cmd.Wait() closes the stdin pipe write-end, causing the stdin io.Copy goroutine to return a broken-pipe error. Since ctx is not cancelled at this point, the ctx.Err() guard did not suppress the log, producing spurious Error-level noise on normal push/clone completions. Downgrade to Debug: a stdin broken-pipe after git exits is expected operational behaviour, not an error worth alerting on. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- pkg/git/service.go | 49 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/pkg/git/service.go b/pkg/git/service.go index 29445cd7e..7ed421042 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 @@ -125,8 +145,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 +159,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 +170,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); erro != nil { + if _, erro := io.Copy(scmd.Stderr, stderr); erro != nil && ctx.Err() == nil { log.Errorf("gitServiceHandler: failed to copy stderr: %v", erro) } }() @@ -176,13 +199,21 @@ func gitServiceHandler(ctx context.Context, svc Service, scmd ServiceCommand) er 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 { - return fmt.Errorf("%w: %s", err, exitErr.Stderr) + retErr = fmt.Errorf("%w: %s", exitErr, exitErr.Stderr) } - } else if errors.Is(err, exec.ErrWaitDelay) && errors.Is(ctx.Err(), context.Canceled) { - // WaitDelay expired after context cancellation — the process was already - // killed; pipe goroutines did not drain within 30s. Not an error. - log.FromContext(ctx).Debug("git pipe drain timed out after cancellation", "service", svc.Name()) + 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 } From 03abe8ef571ea307235967b02669413316912bcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=80=E3=83=8B=E3=82=A8=E3=83=AB=E3=83=BB=E3=82=AB?= =?UTF-8?q?=E3=82=B9=E3=83=88=E3=83=AA=E3=83=AD?= <126793278+dvrd@users.noreply.github.com> Date: Sat, 28 Mar 2026 15:47:27 +0100 Subject: [PATCH 049/148] fix(ssh): create host key directory before generating key (#18) * fix(ssh): create host key directory before generating key When ssh.key_path points to a directory that does not yet exist, the keygen.New call inside wish.WithHostKeyPath fails silently. wish then falls back to a new ephemeral in-memory host key, causing the server's SSH identity to change on every restart and triggering host-key-changed errors for clients. Add os.MkdirAll for the key file's parent directory before building the SSH server options. This mirrors the implicit assumption that the default ssh/ directory exists, and makes any custom key_path work correctly on first start. Closes #779 Co-Authored-By: Claude Sonnet 4.6 * fix(ssh): skip MkdirAll when KeyPath has no directory component filepath.Dir on a bare filename returns "." which would silently make MkdirAll a no-op targeting the process CWD. Guard with dir != "." to skip the call in that case and avoid confusing failure modes. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- pkg/ssh/ssh.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/ssh/ssh.go b/pkg/ssh/ssh.go index 7f261cca2..f80747111 100644 --- a/pkg/ssh/ssh.go +++ b/pkg/ssh/ssh.go @@ -5,6 +5,7 @@ import ( "fmt" "net" "os" + "path/filepath" "strconv" "time" @@ -88,6 +89,19 @@ func NewSSHServer(ctx context.Context) (*SSHServer, error) { ), } + // Ensure the host key directory exists before wish.WithHostKeyPath tries + // to generate the key. If the parent directory is missing, keygen.New + // fails silently inside wish and the server falls back to a new ephemeral + // in-memory key on every restart — causing host-key-changed errors for + // clients on each restart. + if cfg.SSH.KeyPath != "" { + if dir := filepath.Dir(cfg.SSH.KeyPath); dir != "." { + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, fmt.Errorf("create host key directory: %w", err) + } + } + } + opts := []ssh.Option{ ssh.PublicKeyAuth(s.PublicKeyHandler), ssh.KeyboardInteractiveAuth(s.KeyboardInteractiveHandler), From c3c6c32629d60848614f398f2e5fa43adc28cb99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=80=E3=83=8B=E3=82=A8=E3=83=AB=E3=83=BB=E3=82=AB?= =?UTF-8?q?=E3=82=B9=E3=83=88=E3=83=AA=E3=83=AD?= <126793278+dvrd@users.noreply.github.com> Date: Sat, 28 Mar 2026 16:35:11 +0100 Subject: [PATCH 050/148] fix(server): pre-bind listeners; idempotent Shutdown; TLS guard (#645) (#19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(server): surface bind errors immediately by pre-binding listeners Previously, Start() launched all servers as goroutines and called ListenAndServe() inside each one. If a server failed to bind (EACCES on a privileged port, address already in use, etc.) the goroutine returned an error to the errgroup — but the errgroup's derived context was discarded (`errg, _ := errgroup.WithContext`), so the remaining goroutines kept running and errg.Wait() blocked forever. The error was never propagated to the caller and the process appeared healthy. Fix: pre-bind all net.Listeners synchronously before starting any goroutines. If any net.Listen call fails, all already-bound listeners are closed (via deferred cleanup) and the error is returned immediately. Once all binds succeed, goroutines call Serve(ln) with the pre-bound listener. Add Serve(net.Listener) to HTTPServer and StatsServer to support this pattern (SSHServer and GitDaemon already had Serve methods). Closes #645 * fix(server): shut down all servers when any goroutine fails unexpectedly errgroup.WithContext was called but its derived context was discarded (`errg, _ := ...`). When a server goroutine fails at runtime, the errgroup cancels its derived context but nothing observes it, so the other goroutines keep running and errg.Wait() blocks forever. Add a monitor goroutine that waits for the errgroup context to be cancelled, then calls s.Shutdown() to stop all remaining servers so errg.Wait() unblocks and the caller sees the original error. When the parent context (s.ctx) is already done (e.g. SIGTERM), the signal handler in serve.go is already driving shutdown, so we do nothing to avoid calling Shutdown twice. * fix(server): move shutdown monitor outside errgroup to avoid blocking Wait() The previous commit added a monitor goroutine *inside* the errgroup to trigger shutdown when a server goroutine fails. This was wrong: during normal SIGTERM shutdown all server goroutines return nil (ErrServerClosed is filtered), so the errgroup's derived context is never cancelled by an error. The monitor goroutine then blocked on <-gctx.Done() forever and errg.Wait() never returned. Move the monitor to a bare goroutine outside the errgroup so it does not participate in errg.Wait(). errgroup.Wait() cancels gctx on return, so the goroutine always unblocks eventually — no goroutine leak. Normal SIGTERM path: serve.go's signal handler calls s.Shutdown(), all server goroutines exit with ErrServerClosed (filtered → nil), errg.Wait() returns nil immediately, gctx fires, monitor sees s.ctx is cancelled, returns without calling Shutdown again. Unexpected failure path: failing goroutine cancels gctx, monitor fires, s.ctx is not yet cancelled, monitor calls s.Shutdown() to stop remaining servers so errg.Wait() unblocks and the caller sees the error. * fix(server): use context.Cause to guard monitor goroutine The previous guard (s.ctx.Err() != nil) was always false on the SIGTERM path because serve.go's signal handler calls s.Shutdown() directly without cancelling s.ctx. This meant the monitor goroutine always called Shutdown a second time, racing with the signal handler's own call. errgroup.WithContext uses context.WithCancelCause internally: - When a goroutine returns a non-nil error, it calls cancel(err) → context.Cause(gctx) == err (non-nil) → server crashed, Shutdown needed - When Wait() returns normally or the parent is cancelled, it calls cancel(nil) → context.Cause(gctx) == nil → skip, avoid double-Shutdown Closes #645 Co-Authored-By: Claude Sonnet 4.6 * fix(server): address adversarial review findings - server.go: add shutdownOnce to prevent double-Shutdown when SIGTERM and a goroutine failure race; fix comment to accurately describe errgroup cancel semantics (cancel(g.err), not always cancel(nil)) - http.go: guard ServeTLS against TLSConfig without a certificate source (GetCertificate or Certificates); add errors import - stats.go: document that the stats endpoint intentionally omits TLS Closes #645 Co-Authored-By: Claude Sonnet 4.6 * fix(server): make Shutdown idempotent via shutdownOnce Move shutdownOnce protection into Shutdown() itself so every call path — monitor goroutine and serve.go's SIGTERM handler — is covered by the same Once guard. The previous approach only wrapped the monitor path, leaving the SIGTERM path unguarded. Also fix TLS guard in HTTPServer.Serve: add GetConfigForClient to the certificate-source check so SNI-routing configs are not rejected. Closes #645 Co-Authored-By: Claude Sonnet 4.6 * fix(server): propagate shutdown error and guard ListenAndServe TLS - Store shutdownErr on Server so all concurrent Shutdown callers (both monitor goroutine and SIGTERM handler) return the real error; process exits with non-zero when shutdown fails after a crash - Add errgroup version note to context.Cause usage comment - Apply TLS certificate source guard to ListenAndServe (was only in Serve) Closes #645 Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- cmd/soft/serve/server.go | 125 +++++++++++++++++++++++++++++++++++---- pkg/stats/stats.go | 8 +++ pkg/web/http.go | 26 ++++++++ 3 files changed, 146 insertions(+), 13 deletions(-) diff --git a/cmd/soft/serve/server.go b/cmd/soft/serve/server.go index fda090050..3365d77df 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 @@ -111,48 +116,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 +254,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/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/web/http.go b/pkg/web/http.go index 531d02bfd..e5c7ca405 100644 --- a/pkg/web/http.go +++ b/pkg/web/http.go @@ -3,6 +3,8 @@ package web import ( "context" "crypto/tls" + "errors" + "net" "net/http" "time" @@ -48,9 +50,33 @@ func (s *HTTPServer) Close() error { return s.Server.Close() } +// Serve accepts connections on l and serves HTTP requests. +func (s *HTTPServer) Serve(l net.Listener) error { + if s.Server.TLSConfig != nil { + // ServeTLS with empty cert/key paths is only valid when at least + // one certificate source is set on the TLSConfig: Certificates, + // GetCertificate, or GetConfigForClient (which can supply a full + // tls.Config dynamically, e.g. for SNI-based routing). + tlsCfg := s.Server.TLSConfig + if len(tlsCfg.Certificates) == 0 && + tlsCfg.GetCertificate == nil && + tlsCfg.GetConfigForClient == nil { + return errors.New("TLS configured but no certificate source provided (set Certificates, GetCertificate, or GetConfigForClient)") + } + return s.Server.ServeTLS(l, "", "") + } + return s.Server.Serve(l) +} + // ListenAndServe starts the HTTP server. func (s *HTTPServer) ListenAndServe() error { if s.Server.TLSConfig != nil { + tlsCfg := s.Server.TLSConfig + if len(tlsCfg.Certificates) == 0 && + tlsCfg.GetCertificate == nil && + tlsCfg.GetConfigForClient == nil { + return errors.New("TLS configured but no certificate source provided (set Certificates, GetCertificate, or GetConfigForClient)") + } return s.Server.ListenAndServeTLS("", "") } return s.Server.ListenAndServe() From 8b876c1c406a7f4ec8aa35f5f730d1a4da59e5ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=80=E3=83=8B=E3=82=A8=E3=83=AB=E3=83=BB=E3=82=AB?= =?UTF-8?q?=E3=82=B9=E3=83=88=E3=83=AA=E3=83=AD?= <126793278+dvrd@users.noreply.github.com> Date: Sat, 28 Mar 2026 16:51:28 +0100 Subject: [PATCH 051/148] feat(web): add GET /{repo}/raw/{ref}/{filepath} raw blob endpoint (#456) (#20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(web): add GET /{repo}/raw/{ref}/{filepath} endpoint Exposes raw file content over HTTP, closing the parity gap with the SSH `repo blob` command. - Route: GET /{repo}/raw/{ref}/{filepath:.*} - Requires at least ReadOnlyAccess (respects public/private repo settings) - Content-Type inferred from file extension; falls back to text/plain for text files and application/octet-stream for binaries - Accept: application/octet-stream triggers Content-Disposition: attachment so browsers download the file instead of rendering it - Returns 404 for unknown repo/ref/path and for paths that resolve to a directory (tree) rather than a file (blob) Closes #456 Co-Authored-By: Claude Sonnet 4.6 * fix(web): address security review findings on raw blob endpoint - Remove redundant access check from handler; withAccess catch-all already enforces ReadOnlyAccess (404 on insufficient access) for all unrecognised service paths - Sanitize Content-Type: downgrade text/html, text/javascript, SVG, XML and XHTMLvariants to text/plain to prevent stored-XSS - Add X-Content-Type-Options: nosniff on every response - Guard against OOM/DoS: blobs > 32 MiB return 413 instead of being fully buffered into memory - Quote and escape Content-Disposition filename per RFC 6266 §4.3 - Detect binary using already-loaded bytes (gitb.IsBinary) instead of re-running git cat-file via te.File().IsBinary() - Add Cache-Control: no-store to prevent proxy caching of mutable refs Co-Authored-By: Claude Sonnet 4.6 * fix(web): strengthen blob endpoint security — allowlist MIME and belt-and-suspenders size check - Switch sanitizeMIME from a blocklist to an allowlist: only text/plain, application/octet-stream, application/json, image/* (excluding SVG), audio/*, and video/* pass through; everything else (HTML, CSS, JavaScript, SVG, XML, PDF, fonts, multipart) is downgraded to text/plain - Add post-load size check after te.Contents() as a belt-and-suspenders guard for the case where te.Size() silently returns 0 on git error - Fix Content-Length to use FormatInt(int64) instead of Itoa(int) - Document intentional fall-through in withAccess switch for routes that carry no service var (raw blob, go-get) Co-Authored-By: Claude Sonnet 4.6 * fix(web): strip control chars from Content-Disposition filename; add safety comment - Strip ASCII control chars (0x00-0x1F, 0x7F) from blob filename before inserting into Content-Disposition to prevent response-header injection via crafted filenames (e.g. CRLF sequences) - Add comment noting sanitizeMIME has already run before the Accept-override block so future edits know they must maintain that invariant Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- pkg/web/blob.go | 184 ++++++++++++++++++++++++++++++++++++++++++++++++ pkg/web/git.go | 13 ++++ 2 files changed, 197 insertions(+) create mode 100644 pkg/web/blob.go diff --git a/pkg/web/blob.go b/pkg/web/blob.go new file mode 100644 index 000000000..f1a5e9326 --- /dev/null +++ b/pkg/web/blob.go @@ -0,0 +1,184 @@ +package web + +import ( + "bytes" + "mime" + "net/http" + "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: + // - strip ASCII control characters (0x00-0x1F, 0x7F) to prevent + // response-header injection via crafted filenames + // - escape embedded double-quotes + rawName := filepath.Base(filePath) + safeName := strings.Map(func(r rune) rune { + if r < 0x20 || r == 0x7F { + return -1 // drop control character + } + return r + }, rawName) + safeName = strings.ReplaceAll(safeName, `"`, `\"`) + w.Header().Set("Content-Disposition", `attachment; filename="`+safeName+`"`) + } + + // 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