Commit 298ac0b456
Verified · cmc ci/build: success ci/test: success
Layout: unified · split
.gitbay/wiki/Admin.org +3 −1
| @@ -203,7 +203,9 @@ push=. | |||
| 203 | and again before every sync; git then connects only to the addresses | 203 | and again before every sync; git then connects only to the addresses |
| 204 | that were checked (=http.curloptResolve=) and does not follow | 204 | that were checked (=http.curloptResolve=) and does not follow |
| 205 | redirects, so a mirror of a renamed repository fails until its URL | 205 | redirects, so a mirror of a renamed repository fails until its URL |
| 206 | is updated. Needs git 2.37 or later on the server. | 206 | is updated. Needs git 2.37 or later on the server; with an older git |
| 207 | the worker logs an error at start and syncs no mirror, recording the | ||
| 208 | reason on each. Sync ignores the system and global gitconfig. | ||
| 207 | 209 | ||
| 208 | ** [go_import] | 210 | ** [go_import] |
| 209 | Vanity Go module paths, one per line: ="host/module" = "owner/repo"=. | 211 | Vanity Go module paths, one per line: ="host/module" = "owner/repo"=. |
.gitbay/wiki/Architecture/09-Controls.org +1 −1
| @@ -75,7 +75,7 @@ chapter names of OWASP ASVS 4.0 where one fits. | |||
| 75 | 75 | ||
| 76 | | Control | Status | Evidence | | 76 | | Control | Status | Evidence | |
| 77 | |---------------------------------------------+----------+------------------------------------------------------------------| | 77 | |---------------------------------------------+----------+------------------------------------------------------------------| |
| 78 | | SSRF protection on user-supplied URLs | in place | webhooks at save and connect; mirrors at save and sync, git pinned to the checked address (=internal/mirror/mirror.go=) | | 78 | | SSRF protection on user-supplied URLs | partial | webhooks at save and connect; mirrors at save and sync, git pinned to the checked address (=internal/mirror/mirror.go=); =repo import --from= has no address check (#298) | |
| 79 | | Webhook payload integrity | in place | HMAC-SHA256 header | | 79 | | Webhook payload integrity | in place | HMAC-SHA256 header | |
| 80 | | SMTP credentials protected in transit | in place | STARTTLS required for non-local relays, implicit TLS optional (=internal/mail/mail.go=) | | 80 | | SMTP credentials protected in transit | in place | STARTTLS required for non-local relays, implicit TLS optional (=internal/mail/mail.go=) | |
| 81 | | Upload size limits | in place | per-owner storage quota at push (=internal/sshd/sshd.go=); API body 1 MiB | | 81 | | Upload size limits | in place | per-owner storage quota at push (=internal/sshd/sshd.go=); API body 1 MiB | |
.gitbay/wiki/Architecture/10-Known-Gaps.org +1
| @@ -19,6 +19,7 @@ what the 2026-09-27 review found; remove a row when its issue closes. | |||
| 19 | | #273 | Data at rest | CI secrets, webhook secrets and mirror tokens are stored in clear in SQLite | high | | 19 | | #273 | Data at rest | CI secrets, webhook secrets and mirror tokens are stored in clear in SQLite | high | |
| 20 | | #274 | Backups | The local backup archive is not encrypted | medium | | 20 | | #274 | Backups | The local backup archive is not encrypted | medium | |
| 21 | | #275 | Audit | Refused writes are not audited; the audit table is writable by the daemon user | medium | | 21 | | #275 | Audit | Refused writes are not audited; the audit table is writable by the daemon user | medium | |
| 22 | | #298 | SSRF | =repo import --from= fetches without an address check | medium | | ||
| 22 | | #282 | Hook socket | Anything that can open =hook.sock= can act as any user | medium | | 23 | | #282 | Hook socket | Anything that can open =hook.sock= can act as any user | medium | |
| 23 | | #297 | Credentials | A browser session can mint tokens and keys that outlive it | low | | 24 | | #297 | Credentials | A browser session can mint tokens and keys that outlive it | low | |
| 24 | 25 | ||
.gitbay/wiki/Threat-Model.org +6 −4
| @@ -84,16 +84,18 @@ no inbound HMAC. | |||
| 84 | 84 | ||
| 85 | * Network-facing request forgery | 85 | * Network-facing request forgery |
| 86 | 86 | ||
| 87 | Anything that makes the *server* open an outbound connection to a | 87 | Webhook delivery, GitHub-history import =--api-base= and mirror |
| 88 | user-supplied address — webhook delivery, GitHub-history import | 88 | remotes, which make the *server* open an outbound connection to a |
| 89 | =--api-base=, mirror remotes — passes the same SSRF guard: the scheme | 89 | user-supplied address, pass the same SSRF guard: the scheme |
| 90 | must be http/https and, unless =webhooks.allow_local= is set, the | 90 | must be http/https and, unless =webhooks.allow_local= is set, the |
| 91 | resolved address must not be loopback, private, shared | 91 | resolved address must not be loopback, private, shared |
| 92 | (100.64.0.0/10), link-local, or multicast. The webhook dialer re-checks | 92 | (100.64.0.0/10), link-local, or multicast. The webhook dialer re-checks |
| 93 | at connect time, and the mirror worker resolves and checks before each | 93 | at connect time, and the mirror worker resolves and checks before each |
| 94 | sync and pins git to the checked addresses, so a DNS answer that | 94 | sync and pins git to the checked addresses, so a DNS answer that |
| 95 | changes after validation still cannot reach private space. Redirects | 95 | changes after validation still cannot reach private space. Redirects |
| 96 | are never followed. | 96 | are never followed. =repo import --from= is the exception: its clone |
| 97 | checks the scheme but not the address, and follows git's default | ||
| 98 | redirect rule (#298). | ||
| 97 | 99 | ||
| 98 | * Rendering pushed markup | 100 | * Rendering pushed markup |
| 99 | 101 | ||
CHANGELOG.org +3 −1
| @@ -43,7 +43,9 @@ must add =--scope full=. Existing tokens keep their scope. | |||
| 43 | only to them, with redirects off; a URL that now resolves to private | 43 | only to them, with redirects off; a URL that now resolves to private |
| 44 | space, or is not http or https, fails the sync with the reason on | 44 | space, or is not http or https, fails the sync with the reason on |
| 45 | =repo mirror list=. A mirror of a renamed repository that redirects | 45 | =repo mirror list=. A mirror of a renamed repository that redirects |
| 46 | fails until its URL is updated. Needs git 2.37 or later (#279). | 46 | fails until its URL is updated. Sync ignores the system and global |
| 47 | gitconfig. Needs git 2.37 or later: with an older git no mirror | ||
| 48 | syncs, and each records why (#279). | ||
| 47 | - Webhook and mirror targets in 100.64.0.0/10 or on a multicast | 49 | - Webhook and mirror targets in 100.64.0.0/10 or on a multicast |
| 48 | address are refused, as private addresses are (#279). | 50 | address are refused, as private addresses are (#279). |
| 49 | 51 | ||
internal/mirror/mirror.go +42 −1
| @@ -14,6 +14,7 @@ import ( | |||
| 14 | "os" | 14 | "os" |
| 15 | "os/exec" | 15 | "os/exec" |
| 16 | "path/filepath" | 16 | "path/filepath" |
| 17 | "strconv" | ||
| 17 | "strings" | 18 | "strings" |
| 18 | "time" | 19 | "time" |
| 19 | 20 | ||
| @@ -37,6 +38,9 @@ type Worker struct { | |||
| 37 | Tick time.Duration | 38 | Tick time.Duration |
| 38 | // Lookup resolves a mirror's host immediately before each sync. | 39 | // Lookup resolves a mirror's host immediately before each sync. |
| 39 | Lookup func(ctx context.Context, host string) ([]net.IP, error) | 40 | Lookup func(ctx context.Context, host string) ([]net.IP, error) |
| 41 | // gitErr is set when the server's git cannot pin addresses; no | ||
| 42 | // mirror syncs while it is. | ||
| 43 | gitErr error | ||
| 40 | } | 44 | } |
| 41 | 45 | ||
| 42 | func New(st *store.Store, cfg config.Config) *Worker { | 46 | func New(st *store.Store, cfg config.Config) *Worker { |
| @@ -53,6 +57,15 @@ func New(st *store.Store, cfg config.Config) *Worker { | |||
| 53 | } | 57 | } |
| 54 | 58 | ||
| 55 | func (w *Worker) Run(ctx context.Context) { | 59 | func (w *Worker) Run(ctx context.Context) { |
| 60 | out, err := exec.CommandContext(ctx, toolpath.Look("git"), "version").Output() | ||
| 61 | if err != nil { | ||
| 62 | w.gitErr = fmt.Errorf("mirrors disabled: running git version: %v", err) | ||
| 63 | } else { | ||
| 64 | w.gitErr = gitVersionOK(string(out)) | ||
| 65 | } | ||
| 66 | if w.gitErr != nil { | ||
| 67 | slog.Error("mirror: not syncing", "err", w.gitErr) | ||
| 68 | } | ||
| 56 | t := time.NewTicker(w.Tick) | 69 | t := time.NewTicker(w.Tick) |
| 57 | defer t.Stop() | 70 | defer t.Stop() |
| 58 | for { | 71 | for { |
| @@ -73,6 +86,10 @@ func (w *Worker) sweep() { | |||
| 73 | return | 86 | return |
| 74 | } | 87 | } |
| 75 | for _, m := range due { | 88 | for _, m := range due { |
| 89 | if w.gitErr != nil { | ||
| 90 | w.St.SetMirrorResult(m.ID, w.gitErr.Error()) | ||
| 91 | continue | ||
| 92 | } | ||
| 76 | if err := w.sync(m); err != nil { | 93 | if err := w.sync(m); err != nil { |
| 77 | slog.Warn("mirror sync failed", "mirror", m.ID, "url", m.URL, "err", err) | 94 | slog.Warn("mirror sync failed", "mirror", m.ID, "url", m.URL, "err", err) |
| 78 | w.St.SetMirrorResult(m.ID, err.Error()) | 95 | w.St.SetMirrorResult(m.ID, err.Error()) |
| @@ -113,7 +130,10 @@ func (w *Worker) sync(m store.Mirror) error { | |||
| 113 | return err | 130 | return err |
| 114 | } | 131 | } |
| 115 | 132 | ||
| 116 | env := []string{"GIT_TERMINAL_PROMPT=0", "HOME=" + w.Cfg.Server.Root} | 133 | // No system or global gitconfig: a proxy, URL rewrite or redirect |
| 134 | // setting there would take git around the pin. | ||
| 135 | env := []string{"GIT_TERMINAL_PROMPT=0", "HOME=" + w.Cfg.Server.Root, | ||
| 136 | "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null"} | ||
| 117 | if m.Token != "" { | 137 | if m.Token != "" { |
| 118 | askpass := filepath.Join(w.Cfg.Server.Root, "mirror-askpass.sh") | 138 | askpass := filepath.Join(w.Cfg.Server.Root, "mirror-askpass.sh") |
| 119 | if err := os.WriteFile(askpass, []byte(askpassScript), 0o700); err != nil { | 139 | if err := os.WriteFile(askpass, []byte(askpassScript), 0o700); err != nil { |
| @@ -172,3 +192,24 @@ func pinArgs(u *url.URL, ips []net.IP) []string { | |||
| 172 | } | 192 | } |
| 173 | return append(args, "-c", "http.curloptResolve="+host+":"+port+":"+strings.Join(addrs, ",")) | 193 | return append(args, "-c", "http.curloptResolve="+host+":"+port+":"+strings.Join(addrs, ",")) |
| 174 | } | 194 | } |
| 195 | |||
| 196 | // gitVersionOK accepts the output of `git version` for git 2.37 or | ||
| 197 | // later, the first release with http.curloptResolve. An older git | ||
| 198 | // ignores the setting and would resolve the host itself. | ||
| 199 | func gitVersionOK(out string) error { | ||
| 200 | fields := strings.Fields(out) | ||
| 201 | if len(fields) >= 3 && fields[0] == "git" && fields[1] == "version" { | ||
| 202 | parts := strings.Split(fields[2], ".") | ||
| 203 | if len(parts) >= 2 { | ||
| 204 | major, err1 := strconv.Atoi(parts[0]) | ||
| 205 | minor, err2 := strconv.Atoi(parts[1]) | ||
| 206 | if err1 == nil && err2 == nil { | ||
| 207 | if major > 2 || major == 2 && minor >= 37 { | ||
| 208 | return nil | ||
| 209 | } | ||
| 210 | return fmt.Errorf("mirrors disabled: git %s is older than 2.37 and cannot pin mirror addresses", fields[2]) | ||
| 211 | } | ||
| 212 | } | ||
| 213 | } | ||
| 214 | return fmt.Errorf("mirrors disabled: cannot read git version from %q", strings.TrimSpace(out)) | ||
| 215 | } | ||
internal/mirror/mirror_test.go +62
| @@ -111,6 +111,68 @@ func TestSyncConnectsToTheCheckedAddress(t *testing.T) { | |||
| 111 | } | 111 | } |
| 112 | } | 112 | } |
| 113 | 113 | ||
| 114 | // The server account's own gitconfig cannot route git around the pin: | ||
| 115 | // a proxy and a URL rewrite in HOME's config are both ignored. | ||
| 116 | func TestSyncIgnoresGlobalGitConfig(t *testing.T) { | ||
| 117 | remote, sha := upstream(t) | ||
| 118 | u, _ := url.Parse(remote) | ||
| 119 | root := t.TempDir() | ||
| 120 | st, m, dir := local(t, root, "http://mirror.test:"+u.Port()+"/remote.git") | ||
| 121 | conf := "[http]\n\tproxy = http://127.0.0.1:9\n[url \"http://elsewhere.test/\"]\n\tinsteadOf = http://mirror.test:" + u.Port() + "/\n" | ||
| 122 | if err := os.WriteFile(filepath.Join(root, ".gitconfig"), []byte(conf), 0o644); err != nil { | ||
| 123 | t.Fatal(err) | ||
| 124 | } | ||
| 125 | var cfg config.Config | ||
| 126 | cfg.Server.Root = root | ||
| 127 | cfg.Webhooks.AllowLocal = true | ||
| 128 | w := &Worker{St: st, Cfg: cfg, Lookup: func(context.Context, string) ([]net.IP, error) { | ||
| 129 | return []net.IP{net.ParseIP("127.0.0.1")}, nil | ||
| 130 | }} | ||
| 131 | if err := w.sync(m); err != nil { | ||
| 132 | t.Fatal(err) | ||
| 133 | } | ||
| 134 | if got := git(t, dir, "rev-parse", "refs/heads/main"); got != sha { | ||
| 135 | t.Fatalf("main = %s, want %s", got, sha) | ||
| 136 | } | ||
| 137 | } | ||
| 138 | |||
| 139 | // A git too old for http.curloptResolve would ignore the pin; the | ||
| 140 | // sweep refuses to sync and says why on every due mirror. | ||
| 141 | func TestSweepRefusesWithAnOldGit(t *testing.T) { | ||
| 142 | root := t.TempDir() | ||
| 143 | st, m, _ := local(t, root, "https://mirror.test/x.git") | ||
| 144 | var cfg config.Config | ||
| 145 | cfg.Server.Root = root | ||
| 146 | cfg.Mirrors.PullIntervalMinutes = 15 | ||
| 147 | w := &Worker{St: st, Cfg: cfg, Lookup: func(context.Context, string) ([]net.IP, error) { | ||
| 148 | t.Fatal("looked up a host with an old git") | ||
| 149 | return nil, nil | ||
| 150 | }} | ||
| 151 | w.gitErr = gitVersionOK("git version 2.36.1") | ||
| 152 | w.sweep() | ||
| 153 | ms, err := st.ListMirrors(m.RepoID) | ||
| 154 | if err != nil || len(ms) != 1 { | ||
| 155 | t.Fatalf("mirrors: %v %v", ms, err) | ||
| 156 | } | ||
| 157 | if !strings.Contains(ms[0].LastError, "2.37") { | ||
| 158 | t.Fatalf("last error = %q", ms[0].LastError) | ||
| 159 | } | ||
| 160 | } | ||
| 161 | |||
| 162 | func TestGitVersionOK(t *testing.T) { | ||
| 163 | for _, s := range []string{"git version 2.37.0", "git version 2.47.3", "git version 2.39.5 (Apple Git-154)", | ||
| 164 | "git version 2.45.2.windows.1", "git version 3.0.0\n"} { | ||
| 165 | if err := gitVersionOK(s); err != nil { | ||
| 166 | t.Errorf("%q: %v", s, err) | ||
| 167 | } | ||
| 168 | } | ||
| 169 | for _, s := range []string{"git version 2.36.9", "git version 1.99.0", "git version 2", "nonsense", ""} { | ||
| 170 | if err := gitVersionOK(s); err == nil { | ||
| 171 | t.Errorf("%q accepted", s) | ||
| 172 | } | ||
| 173 | } | ||
| 174 | } | ||
| 175 | |||
| 114 | // The URL passed the check when it was saved; the answer at sync time | 176 | // The URL passed the check when it was saved; the answer at sync time |
| 115 | // is what counts. | 177 | // is what counts. |
| 116 | func TestSyncRefusesAPrivateAddressAtSyncTime(t *testing.T) { | 178 | func TestSyncRefusesAPrivateAddressAtSyncTime(t *testing.T) { |
internal/webhook/webhook.go +7 −6
| @@ -20,19 +20,20 @@ import ( | |||
| 20 | "gitbay.org/gitbay/internal/store" | 20 | "gitbay.org/gitbay/internal/store" |
| 21 | ) | 21 | ) |
| 22 | 22 | ||
| 23 | // ValidateURL rejects URLs a webhook must not target: non-HTTP schemes and, | 23 | // ValidateURL rejects URLs the server must not connect to (SSRF): non-HTTP |
| 24 | // unless allowLocal, anything resolving to loopback, private, or link-local | 24 | // schemes and, unless allowLocal, anything resolving to a loopback, |
| 25 | // addresses (SSRF). | 25 | // private, shared (100.64.0.0/10), link-local, multicast or unspecified |
| 26 | // address. Webhooks, mirrors and issue import use it. | ||
| 26 | func ValidateURL(raw string, allowLocal bool) error { | 27 | func ValidateURL(raw string, allowLocal bool) error { |
| 27 | u, err := url.Parse(raw) | 28 | u, err := url.Parse(raw) |
| 28 | if err != nil { | 29 | if err != nil { |
| 29 | return fmt.Errorf("invalid URL: %w", err) | 30 | return fmt.Errorf("invalid URL: %w", err) |
| 30 | } | 31 | } |
| 31 | if u.Scheme != "http" && u.Scheme != "https" { | 32 | if u.Scheme != "http" && u.Scheme != "https" { |
| 32 | return fmt.Errorf("webhook URLs must be http or https") | 33 | return fmt.Errorf("URLs must be http or https") |
| 33 | } | 34 | } |
| 34 | if u.Hostname() == "" { | 35 | if u.Hostname() == "" { |
| 35 | return fmt.Errorf("webhook URL has no host") | 36 | return fmt.Errorf("URL has no host") |
| 36 | } | 37 | } |
| 37 | if allowLocal { | 38 | if allowLocal { |
| 38 | return nil | 39 | return nil |
| @@ -42,7 +43,7 @@ func ValidateURL(raw string, allowLocal bool) error { | |||
| 42 | return fmt.Errorf("cannot resolve %s: %w", u.Hostname(), err) | 43 | return fmt.Errorf("cannot resolve %s: %w", u.Hostname(), err) |
| 43 | } | 44 | } |
| 44 | if err := CheckAddrs(u.Hostname(), ips, allowLocal); err != nil { | 45 | if err := CheckAddrs(u.Hostname(), ips, allowLocal); err != nil { |
| 45 | return fmt.Errorf("webhook target %s resolves to a private or local address; refusing (SSRF)", u.Hostname()) | 46 | return fmt.Errorf("target %s resolves to a private or local address; refusing (SSRF)", u.Hostname()) |
| 46 | } | 47 | } |
| 47 | return nil | 48 | return nil |
| 48 | } | 49 | } |