5b307b4da4db285f797cc028b381ba4d3d853019

Author
Kieran Klukas <kieran@dunkirk.sh>
Committer
Kieran Klukas <kieran@dunkirk.sh>
Date

Message

fix: keep a repository private after its visibility changes

Repository details were cached in memory with no expiry and trusted for
access checks. Making a repository private cleared the cache before
saving the change, so a request arriving in between could cache the old
"public" state, leaving the repository readable without logging in until
the server restarted. Servers sharing one database never saw each
other's changes at all.

The cache is removed. A lookup costs tens of microseconds against
SQLite, which is small next to the git work behind every request, and
access checks now always read the saved state. (GHSA-f3fj-9625-rv3m)

💘 Generated with Crush

Assisted-by: Crush:claude-opus-5-5

Diff

This diff is truncated to protect this page.

  1diff --git a/go.mod b/go.mod
  2index d20b2543ed91e024530fc9b3f8f473fcd911d76f..dddf8b6612effec17ba6537b7148f870f7aa5434 100644
  3--- a/go.mod
  4+++ b/go.mod
  5@@ -27,7 +27,6 @@ require (
  6 	github.com/google/uuid v1.6.0
  7 	github.com/gorilla/handlers v1.5.2
  8 	github.com/gorilla/mux v1.8.1
  9-	github.com/hashicorp/golang-lru/v2 v2.0.7
 10 	github.com/jmoiron/sqlx v1.4.0
 11 	github.com/lib/pq v1.12.3
 12 	github.com/lrstanley/bubblezone/v2 v2.0.0
 13diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go
 14index 95d45ec2d90a0749ad7028c4b9a164f64694752a..80a29cd6e15cd4111c37a61134f13c792ae27ac8 100644
 15--- a/pkg/backend/backend.go
 16+++ b/pkg/backend/backend.go
 17@@ -18,7 +18,6 @@ type Backend struct {
 18 	db      *db.DB
 19 	store   store.Store
 20 	logger  *log.Logger
 21-	cache   *cache
 22 	manager *task.Manager
 23 }
 24 
 25@@ -34,9 +33,5 @@ func New(ctx context.Context, cfg *config.Config, db *db.DB, st store.Store) *Ba
 26 		manager: task.NewManager(ctx),
 27 	}
 28 
 29-	// TODO: implement a proper caching interface
 30-	cache := newCache(b, 1000)
 31-	b.cache = cache
 32-
 33 	return b
 34 }
 35diff --git a/pkg/backend/cache.go b/pkg/backend/cache.go
 36deleted file mode 100644
 37index 8d99401dd94fee4c02ce145693de8c53ee5bfaae..0000000000000000000000000000000000000000
 38--- a/pkg/backend/cache.go
 39+++ /dev/null
 40@@ -1,35 +0,0 @@
 41-package backend
 42-
 43-import lru "github.com/hashicorp/golang-lru/v2"
 44-
 45-// TODO: implement a caching interface.
 46-type cache struct {
 47-	b     *Backend
 48-	repos *lru.Cache[string, *repo]
 49-}
 50-
 51-func newCache(b *Backend, size int) *cache {
 52-	if size <= 0 {
 53-		size = 1
 54-	}
 55-	c := &cache{b: b}
 56-	cache, _ := lru.New[string, *repo](size)
 57-	c.repos = cache
 58-	return c
 59-}
 60-
 61-func (c *cache) Get(repo string) (*repo, bool) {
 62-	return c.repos.Get(repo)
 63-}
 64-
 65-func (c *cache) Set(repo string, r *repo) {
 66-	c.repos.Add(repo, r)
 67-}
 68-
 69-func (c *cache) Delete(repo string) {
 70-	c.repos.Remove(repo)
 71-}
 72-
 73-func (c *cache) Len() int {
 74-	return c.repos.Len()
 75-}
 76diff --git a/pkg/backend/repo.go b/pkg/backend/repo.go
 77index 435f9696758cc7485f2deeb437aa15587b380ae9..98be7a8a77315b0d3da08a251270d5ed520cd108 100644
 78--- a/pkg/backend/repo.go
 79+++ b/pkg/backend/repo.go
 80@@ -307,9 +307,6 @@ func (d *Backend) DeleteRepository(ctx context.Context, name string) error {
 81 	}
 82 
 83 	if err := d.db.TransactionContext(ctx, func(tx *db.Tx) error {
 84-		// Delete repo from cache
 85-		defer d.cache.Delete(name)
 86-
 87 		repom, dberr := d.store.GetRepoByName(ctx, tx, name)
 88 		_, ferr := os.Stat(rp)
 89 		if dberr != nil && ferr != nil {
 90@@ -414,9 +411,6 @@ func (d *Backend) RenameRepository(ctx context.Context, oldName string, newName
 91 	}
 92 
 93 	if err := d.db.TransactionContext(ctx, func(tx *db.Tx) error {
 94-		// Delete cache
 95-		defer d.cache.Delete(oldName)
 96-
 97 		if err := d.store.SetRepoNameByName(ctx, tx, oldName, newName); err != nil {
 98 			return err
 99 		}
100@@ -464,9 +458,6 @@ func (d *Backend) Repositories(ctx context.Context) ([]proto.Repository, error)
101 				repo: m,
102 			}
103 
104-			// Cache repositories
105-			d.cache.Set(m.Name, r)
106-
107 			repos = append(repos, r)
108 		}
109 
110@@ -485,10 +476,6 @@ func (d *Backend) Repository(ctx context.Context, name string) (proto.Repository
111 	var m models.Repo
112 	name = utils.SanitizeRepo(name)
113 
114-	if r, ok := d.cache.Get(name); ok && r != nil {
115-		return r, nil
116-	}
117-
118 	rp := filepath.Join(d.repoPath(name))
119 	if _, err := os.Stat(rp); err != nil {
120 		if !errors.Is(err, fs.ErrNotExist) {
121@@ -514,9 +501,6 @@ func (d *Backend) Repository(ctx context.Context, name string) (proto.Repository
122 		repo: m,
123 	}
124 
125-	// Add to cache
126-	d.cache.Set(name, r)
127-
128 	return r, nil
129 }
130 
131@@ -610,9 +594,6 @@ func (d *Backend) ProjectName(ctx context.Context, name string) (string, error)
132 func (d *Backend) SetHidden(ctx context.Context, name string, hidden bool) error {
133 	name = utils.SanitizeRepo(name)
134 
135-	// Delete cache
136-	d.cache.Delete(name)
137-
138 	return db.WrapError(d.db.TransactionContext(ctx, func(tx *db.Tx) error {
139 		return d.store.SetRepoIsHiddenByName(ctx, tx, name, hidden)
140 	}))
141@@ -625,9 +606,6 @@ func (d *Backend) SetMirror(ctx context.Context, name string, mirror bool) error
142 	name = utils.SanitizeRepo(name)
143 	rp := filepath.Join(d.repoPath(name))
144 
145-	// Delete cache
146-	d.cache.Delete(name)
147-
148 	return db.WrapError(d.db.TransactionContext(ctx, func(tx *db.Tx) error {
149 		// Update git config
150 		r, err := git.Open(rp)
151@@ -689,9 +667,6 @@ func (d *Backend) SetDescription(ctx context.Context, name string, desc string)
152 	desc = utils.Sanitize(desc)
153 	rp := filepath.Join(d.repoPath(name))
154 
155-	// Delete cache
156-	d.cache.Delete(name)
157-
158 	return d.db.TransactionContext(ctx, func(tx *db.Tx) error {
159 		if err := os.WriteFile(filepath.Join(rp, "description"), []byte(desc), fs.ModePerm); err != nil {
160 			d.logger.Error("failed to write description", "repo", name, "err", err)
161@@ -709,9 +684,6 @@ func (d *Backend) SetPrivate(ctx context.Context, name string, private bool) err
162 	name = utils.SanitizeRepo(name)
163 	rp := filepath.Join(d.repoPath(name))
164 
165-	// Delete cache
166-	d.cache.Delete(name)
167-
168 	if err := db.WrapError(
169 		d.db.TransactionContext(ctx, func(tx *db.Tx) error {
170 			fp := filepath.Join(rp, "git-daemon-export-ok")
171@@ -762,9 +734,6 @@ func (d *Backend) SetProjectName(ctx context.Context, repo string, name string)
172 	repo = utils.SanitizeRepo(repo)
173 	name = utils.Sanitize(name)
174 
175-	// Delete cache
176-	d.cache.Delete(repo)
177-
178 	return db.WrapError(
179 		d.db.TransactionContext(ctx, func(tx *db.Tx) error {
180diff --git a/pkg/backend/repo_visibility_test.go b/pkg/backend/repo_visibility_test.go
181new file mode 100644
182index 0000000000000000000000000000000000000000..ba3bedc6e7bc52695d8e41eae9988d075edb1860
183--- /dev/null
184+++ b/pkg/backend/repo_visibility_test.go
185@@ -0,0 +1,33 @@
186+package backend
187+
188+import (
189+	"context"
190+	"testing"
191+
192+	"github.com/charmbracelet/soft-serve/pkg/access"
193+	"github.com/charmbracelet/soft-serve/pkg/db"
194+	"github.com/charmbracelet/soft-serve/pkg/proto"
195+	"github.com/matryer/is"
196+)
197+
198+// TestRepositoryReflectsDatabase verifies that access checks see a
199+// visibility change as soon as it is committed, whoever committed it
200+// (GHSA-f3fj-9625-rv3m).
201+func TestRepositoryReflectsDatabase(t *testing.T) {
202+	is := is.New(t)
203+	be, _ := newTestBackend(t)
204+	ctx := context.Background()
205+
206+	_, err := be.CreateRepository(ctx, "repo", nil, proto.RepositoryOptions{})
207+	is.NoErr(err)
208+	is.Equal(be.AccessLevelForUser(ctx, "repo", nil), access.ReadOnlyAccess)
209+
210+	is.NoErr(be.db.TransactionContext(ctx, func(tx *db.Tx) error {
211+		return be.store.SetRepoIsPrivateByName(ctx, tx, "repo", true)
212+	}))
213+
214+	r, err := be.Repository(ctx, "repo")
215+	is.NoErr(err)
216+	is.True(r.IsPrivate())
217+	is.Equal(be.AccessLevelForUser(ctx, "repo", nil), access.NoAccess)
218+}