35103b63e75700bedf0d4d7e65afbae3bfcb9682

Author
Kieran Klukas <kieran@dunkirk.sh>
Committer
Christian Rocha <christian@rocha.is>
Date

Message

refactor: make anon-access config a typed tri-state override

AnonAccess was a string with "" as the "unset" sentinel, while
AllowKeyless used *bool with nil. Both are the same concept (tri-state
config overrides) but were modeled differently, requiring manual
validation in Validate() and a ParseAccessLevel call on every read.

Switch AnonAccess to *access.AccessLevel. Since AccessLevel already
implements TextUnmarshaler, invalid values are rejected at parse time
for free. The manual Validate() block, the hot-path re-parse, and the
"" sentinel all disappear. Both overrides now use the same nil-means-
unset pattern, and Environ() only emits ANON_ACCESS when explicitly set.

💘 Generated with Crush

Assisted-by: Crush:qwen3.8-max-preview

Diff

  1diff --git a/cmd/soft/serve/server_test.go b/cmd/soft/serve/server_test.go
  2index ce7c9745094e1fb1672f008181dfc3a79bca2172..37d5eb7b3f0ff66429358382a8e4d76636bbc285 100644
  3--- a/cmd/soft/serve/server_test.go
  4+++ b/cmd/soft/serve/server_test.go
  5@@ -66,7 +66,7 @@ func TestWarnIfAnonAdminAccess(t *testing.T) {
  6 
  7 			allow := c.allowKeyless
  8 			cfg.AllowKeyless = &allow
  9-			cfg.AnonAccess = c.anonAccess.String()
 10+			cfg.AnonAccess = &c.anonAccess
 11 
 12 			var buf bytes.Buffer
 13 			logger := log.New(&buf)
 14diff --git a/pkg/backend/settings.go b/pkg/backend/settings.go
 15index c78cf25f4c9044b632fa9c391a769be4494e8c1a..9e169a86851d1d80439ca1db239d606d8740bc51 100644
 16--- a/pkg/backend/settings.go
 17+++ b/pkg/backend/settings.go
 18@@ -52,8 +52,8 @@ func (b *Backend) SetAllowKeyless(ctx context.Context, allow bool) error {
 19 //
 20 // It implements backend.Backend.
 21 func (b *Backend) AnonAccess(ctx context.Context) access.AccessLevel {
 22-	if b.cfg.AnonAccess != "" {
 23-		return access.ParseAccessLevel(b.cfg.AnonAccess)
 24+	if b.cfg.AnonAccess != nil {
 25+		return *b.cfg.AnonAccess
 26 	}
 27 
 28 	var level access.AccessLevel
 29diff --git a/pkg/backend/settings_test.go b/pkg/backend/settings_test.go
 30index bba2b93fb62f743dfc88c06fb5aef328083ec47c..e78b6fb63b4e35df9362bf2f3bc115df333ef25f 100644
 31--- a/pkg/backend/settings_test.go
 32+++ b/pkg/backend/settings_test.go
 33@@ -54,12 +54,13 @@ func TestAnonAccessConfigOverride(t *testing.T) {
 34 	is.Equal(be.AnonAccess(ctx), access.ReadWriteAccess)
 35 
 36 	// A config override takes precedence over whatever is in the DB.
 37-	cfg.AnonAccess = access.AdminAccess.String()
 38+	admin := access.AdminAccess
 39+	cfg.AnonAccess = &admin
 40 	is.Equal(be.AnonAccess(ctx), access.AdminAccess)
 41 
 42 	// The DB value is unchanged underneath the override: once the override
 43 	// is cleared, the last DB write is what's returned again.
 44-	cfg.AnonAccess = ""
 45+	cfg.AnonAccess = nil
 46 	is.Equal(be.AnonAccess(ctx), access.ReadWriteAccess)
 47 }
 48 
 49diff --git a/pkg/config/config.go b/pkg/config/config.go
 50index 288c831ce307979ad8892fd135570285116a99c9..f17e6afbd229a49d034b5299f632e21fbd6b133e 100644
 51--- a/pkg/config/config.go
 52+++ b/pkg/config/config.go
 53@@ -174,13 +174,14 @@ type Config struct {
 54 
 55 	// AnonAccess overrides the access level for anonymous users.
 56 	//
 57-	// If set, this takes precedence over the "anon-access" value stored in
 58-	// the database (see the `settings` command) on every read, for as long
 59-	// as it remains set. It is not a one-time default: it behaves like
 60-	// InitialAdminKeys, not like a seed value. This is intended for
 61-	// scripted, non-production bootstrapping only; it is not validated for
 62-	// safety beyond being a recognized access level.
 63-	AnonAccess string `env:"ANON_ACCESS" yaml:"anon_access"`
 64+	// If set (non-nil), this takes precedence over the "anon-access" value
 65+	// stored in the database (see the `settings` command) on every read, for
 66+	// as long as it remains set. It is not a one-time default: it behaves
 67+	// like InitialAdminKeys, not like a seed value. Invalid values are
 68+	// rejected at parse time by AccessLevel's TextUnmarshaler.
 69+	//
 70+	// This is intended for scripted, non-production bootstrapping only.
 71+	AnonAccess *access.AccessLevel `env:"ANON_ACCESS" yaml:"anon_access"`
 72 
 73 	// AllowKeyless overrides whether keyless (no public key) connections
 74 	// are allowed.
 75@@ -209,7 +210,6 @@ func (c *Config) Environ() []string {
 76 		fmt.Sprintf("SOFT_SERVE_DATA_PATH=%s", c.DataPath),
 77 		fmt.Sprintf("SOFT_SERVE_NAME=%s", c.Name),
 78 		fmt.Sprintf("SOFT_SERVE_INITIAL_ADMIN_KEYS=%s", strings.Join(c.InitialAdminKeys, "\n")),
 79-		fmt.Sprintf("SOFT_SERVE_ANON_ACCESS=%s", c.AnonAccess),
 80 		fmt.Sprintf("SOFT_SERVE_SSH_ENABLED=%t", c.SSH.Enabled),
 81 		fmt.Sprintf("SOFT_SERVE_SSH_LISTEN_ADDR=%s", c.SSH.ListenAddr),
 82 		fmt.Sprintf("SOFT_SERVE_SSH_PUBLIC_URL=%s", c.SSH.PublicURL),
 83@@ -242,9 +242,13 @@ func (c *Config) Environ() []string {
 84 		fmt.Sprintf("SOFT_SERVE_JOBS_MIRROR_PULL=%s", c.Jobs.MirrorPull),
 85 	}...)
 86 
 87-	// AllowKeyless is a tri-state override: only emit it when explicitly
 88-	// set, so a subprocess parsing these envs sees the same "unset" state
 89-	// (rather than an empty string coercing to false).
 90+	// AnonAccess and AllowKeyless are tri-state overrides: only emit them
 91+	// when explicitly set, so a subprocess parsing these envs sees the same
 92+	// "unset" state (rather than an empty string coercing to a zero value).
 93+	if c.AnonAccess != nil {
 94+		envs = append(envs, fmt.Sprintf("SOFT_SERVE_ANON_ACCESS=%s", c.AnonAccess.String()))
 95+	}
 96+
 97 	if c.AllowKeyless != nil {
 98 		envs = append(envs, fmt.Sprintf("SOFT_SERVE_ALLOW_KEYLESS=%t", *c.AllowKeyless))
 99 	}
100@@ -472,10 +476,6 @@ func (c *Config) Validate() error {
101 
102 	c.HTTP.CORS.AllowedOrigins = append([]string{c.HTTP.PublicURL}, c.HTTP.CORS.AllowedOrigins...)
103 
104-	if c.AnonAccess != "" && access.ParseAccessLevel(c.AnonAccess) < 0 {
105-		return fmt.Errorf("invalid anon-access level %q", c.AnonAccess)
106-	}
107-
108 	return nil
109 }
110 
111diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
112index b7cdc52b8fe802530941d64ee004c54fe5a4ea2b..990c9a83004b22778b6d082958ef100a7a06b612 100644
113--- a/pkg/config/config_test.go
114+++ b/pkg/config/config_test.go
115@@ -130,8 +130,8 @@ func TestAnonAccessEnvUnsetByDefault(t *testing.T) {
116 	is := is.New(t)
117 	cfg := DefaultConfig()
118 	is.NoErr(cfg.ParseEnv())
119-	// Empty string is the "no override" sentinel.
120-	is.Equal(cfg.AnonAccess, "")
121+	// nil is the "no override" sentinel.
122+	is.True(cfg.AnonAccess == nil)
123 }
124 
125 func TestParseAnonAccessEnv(t *testing.T) {
126@@ -142,16 +142,20 @@ func TestParseAnonAccessEnv(t *testing.T) {
127 	})
128 	cfg := DefaultConfig()
129 	is.NoErr(cfg.ParseEnv())
130-	is.Equal(cfg.AnonAccess, access.AdminAccess.String())
131+	is.True(cfg.AnonAccess != nil)
132+	is.Equal(*cfg.AnonAccess, access.AdminAccess)
133 }
134 
135-func TestValidateRejectsInvalidAnonAccess(t *testing.T) {
136+// An invalid anon-access level is rejected at parse time by AccessLevel's
137+// TextUnmarshaler, so it never reaches Validate as a bad value.
138+func TestParseRejectsInvalidAnonAccess(t *testing.T) {
139 	is := is.New(t)
140-	cfg := &Config{
141-		DataPath:   t.TempDir(),
142-		AnonAccess: "not-a-real-access-level",
143-	}
144-	err := cfg.Validate()
145+	is.NoErr(os.Setenv("SOFT_SERVE_ANON_ACCESS", "not-a-real-access-level"))
146+	t.Cleanup(func() {
147+		is.NoErr(os.Unsetenv("SOFT_SERVE_ANON_ACCESS"))
148+	})
149+	cfg := DefaultConfig()
150+	err := cfg.ParseEnv()
151 	is.True(err != nil)
152 }
153 
154diff --git a/pkg/ssh/cmd/settings.go b/pkg/ssh/cmd/settings.go
155index c9db032b3640641cec5ecd6d11f21f62fb235c66..297ba195a4667a12ea4a7f533eac39f7a252c2cb 100644
156--- a/pkg/ssh/cmd/settings.go
157+++ b/pkg/ssh/cmd/settings.go
158@@ -97,12 +97,12 @@ func warnIfAllowKeylessOverridden(cmd *cobra.Command, cfg *config.Config) {
159 // the database. Without this, an admin changing the setting via this
160 // command would have no way to know their change has no effect.
161 func warnIfAnonAccessOverridden(cmd *cobra.Command, cfg *config.Config) {
162-	if cfg == nil || cfg.AnonAccess == "" {
163+	if cfg == nil || cfg.AnonAccess == nil {
164 		return
165 	}
166 
167 	fmt.Fprintf(cmd.ErrOrStderr(),
168 		"Warning: anon-access is set to %q by server config and takes precedence over this change. "+
169 			"The database was updated, but it will have no effect until the config override is removed.\n",
170-		cfg.AnonAccess)
171+		cfg.AnonAccess.String())
172 }
173diff --git a/pkg/ssh/cmd/settings_test.go b/pkg/ssh/cmd/settings_test.go
174index 59eba202287429b938ec229b9a0788b993fa0777..2aba319e9191032a534cad6b352d5a646898d49e 100644
175--- a/pkg/ssh/cmd/settings_test.go
176+++ b/pkg/ssh/cmd/settings_test.go
177@@ -6,6 +6,7 @@ import (
178 	"strings"
179 	"testing"
180 
181+	"github.com/charmbracelet/soft-serve/pkg/access"
182 	"github.com/charmbracelet/soft-serve/pkg/backend"
183 	"github.com/charmbracelet/soft-serve/pkg/config"
184 	"github.com/charmbracelet/soft-serve/pkg/db"
185@@ -75,7 +76,8 @@ func TestSettingsAnonAccessWarnsOnConfigOverride(t *testing.T) {
186 	// With a config override active, the write still succeeds (so it takes
187 	// effect if the override is later removed), but must warn loudly that
188 	// it currently has no effect.
189-	cfg.AnonAccess = "admin-access"
190+	adminAccess := access.AdminAccess
191+	cfg.AnonAccess = &adminAccess
192 	_, stderr, err = runSettings(t, ctx, "anon-access", "no-access")
193 	is.NoErr(err)
194 	if !strings.Contains(stderr, "override") {