Skip to content

Commit 7c04e4b

Browse files
aprimakinaclaude
andcommitted
feat(config): enable read-only mode by default for new installations
Flip the read_only default to true so fresh installs refuse mutating operations (CLI service commands, MCP write tools) and open database sessions read-only until the user opts in with `tiger config set read_only false`. Existing installations are grandfathered: older CLI versions only wrote read_only to the config file when set explicitly, so a config file without the key predates the default flip and MigrateReadOnly (same in-memory-shim pattern as MigrateVersionCheck) resolves it to false. To keep that detection sound, every config file written by Set/Unset/Reset now records read_only explicitly; unsetting or resetting restores the current default (true). The integration test suite sets TIGER_READ_ONLY=false since it exercises real mutating commands, and ErrReadOnly now tells users how to disable read-only mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent dea83c8 commit 7c04e4b

8 files changed

Lines changed: 211 additions & 7 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,8 @@ The Tiger MCP server provides AI assistants with programmatic access to Tiger re
322322

323323
Write/destructive MCP tool handlers and CLI command `RunE` functions must call `common.CheckReadOnly(cfg.Config)` (defined in `internal/tiger/common/errors.go`) immediately after `common.LoadConfig(ctx)`. When `cfg.ReadOnly` is `true`, the call returns `common.ErrReadOnly` and the API client is never invoked. The gated CLI commands today are `service create`, `service fork`, `service start`, `service stop`, `service resize`, `service update-password`, and `service delete`.
324324

325+
`read_only` defaults to `true` for new installations only: a config file without the key predates the default flip and is grandfathered to `false` by `MigrateReadOnly` (same in-memory-shim pattern as `MigrateVersionCheck`). To keep that detection sound, every config file written by `Set`/`Unset`/`Reset` records `read_only` explicitly (see `materializeReadOnly`) — preserve this invariant in any new config-file write path. In tests, a config file written via `config.UseTestConfig` without `read_only` therefore behaves like a grandfathered install (`read_only=false`).
326+
325327
**Tool Definition Pattern:**
326328

327329
When defining MCP tools, we use a pattern that balances type safety with schema flexibility:

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ All configuration options can be set via `tiger config set <key> <value>`:
245245
- `docs_mcp` - Enable/disable docs MCP proxy (default: `true`)
246246
- `output` - Output format: `json`, `yaml`, or `table` (default: `table`)
247247
- `password_storage` - Password storage method: `keyring`, `pgpass`, or `none` (default: `keyring`)
248-
- `read_only` - When `true`, mutating operations are refused: the `tiger service create`/`fork`/`start`/`stop`/`resize`/`update-password`/`delete` CLI commands and their MCP equivalents return an error, and `tiger db connect`, `tiger db connection-string`, and the `db_execute_query` MCP tool open the database session in Tiger Cloud's immutable read-only mode (writes and DDL are rejected by the server). Read commands/tools are unaffected. Default: `false`.
248+
- `read_only` - When `true`, mutating operations are refused: the `tiger service create`/`fork`/`start`/`stop`/`resize`/`update-password`/`delete` CLI commands and their MCP equivalents return an error, and `tiger db connect`, `tiger db connection-string`, and the `db_execute_query` MCP tool open the database session in Tiger Cloud's immutable read-only mode (writes and DDL are rejected by the server). Read commands/tools are unaffected. Default: `true` for new installations; config files created by older CLI versions keep the previous default of `false`. Set `read_only` to `false` to enable mutating operations.
249249
- `service_id` - Default service ID
250250
- `version_check` - When `true`, the CLI checks for a newer version on each invocation (in an interactive terminal) and prints a notice if one is available. Set to `false` to disable. Default: `true`.
251251

@@ -260,7 +260,7 @@ Environment variables override configuration file values. All variables use the
260260
- `TIGER_DOCS_MCP` - Enable/disable docs MCP proxy
261261
- `TIGER_OUTPUT` - Output format: `json`, `yaml`, or `table`
262262
- `TIGER_PASSWORD_STORAGE` - Password storage method: `keyring`, `pgpass`, or `none`
263-
- `TIGER_READ_ONLY` - When `true`, write/destructive CLI commands and Tiger MCP tools refuse to run, and `db_execute_query` runs against a read-only database connection
263+
- `TIGER_READ_ONLY` - When `true` (the default for new installations), write/destructive CLI commands and Tiger MCP tools refuse to run, and `db_execute_query` runs against a read-only database connection
264264
- `TIGER_PUBLIC_KEY` - Public key to use for authentication (takes priority over stored credentials)
265265
- `TIGER_SECRET_KEY` - Secret key to use for authentication (takes priority over stored credentials)
266266
- `TIGER_SERVICE_ID` - Default service ID

internal/tiger/cmd/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ func buildConfigShowCmd() *cobra.Command {
5252
return err
5353
}
5454
config.MigrateVersionCheck(v)
55+
if !noDefaults {
56+
// Grandfathered read_only is effectively a default, so hide
57+
// it with the other defaults.
58+
config.MigrateReadOnly(v)
59+
}
5560

5661
cfgOut, err := config.ForOutputFromViper(v)
5762
if err != nil {

internal/tiger/cmd/integration_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ func setupIntegrationTest(t *testing.T) string {
3434
// Disable analytics for integration tests to avoid tracking test events
3535
os.Setenv("TIGER_ANALYTICS", "false")
3636

37+
// Disable read-only mode (default: true) so mutating commands can run
38+
os.Setenv("TIGER_READ_ONLY", "false")
39+
3740
// Reset global config and viper to ensure test isolation
3841
config.ResetGlobalConfig()
3942

@@ -61,6 +64,7 @@ func setupIntegrationTest(t *testing.T) string {
6164
// Clean up environment variables BEFORE cleaning up file system
6265
os.Unsetenv("TIGER_CONFIG_DIR")
6366
os.Unsetenv("TIGER_ANALYTICS")
67+
os.Unsetenv("TIGER_READ_ONLY")
6468
// Then clean up file system
6569
os.RemoveAll(tmpDir)
6670
})

internal/tiger/config/config.go

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ const (
6565
DefaultGatewayURL = "https://console.cloud.tigerdata.com/api"
6666
DefaultOutput = "table"
6767
DefaultPasswordStorage = "keyring"
68-
DefaultReadOnly = false
68+
DefaultReadOnly = true
6969
DefaultReleasesURL = "https://cli.tigerdata.com"
7070
DefaultVersionCheck = true
7171

@@ -137,6 +137,7 @@ func SetupViper(configDir string) error {
137137
}
138138

139139
MigrateVersionCheck(v)
140+
MigrateReadOnly(v)
140141
return nil
141142
}
142143

@@ -159,6 +160,36 @@ func MigrateVersionCheck(v *viper.Viper) {
159160
}
160161
}
161162

163+
// MigrateReadOnly grandfathers installations that predate read-only-by-default.
164+
// Older CLI versions only wrote read_only to the config file when the user set
165+
// it explicitly (current versions always record it; see materializeReadOnly),
166+
// so a config file without the key predates the default flip and keeps its old
167+
// effective value (false) rather than breaking mutating commands on upgrade.
168+
// Fresh installations have no config file and get the read-only default.
169+
//
170+
// Applied via SetDefault, so an explicit config file value or TIGER_READ_ONLY
171+
// env var still wins. Like MigrateVersionCheck, this is an in-memory shim.
172+
func MigrateReadOnly(v *viper.Viper) {
173+
if _, err := os.Stat(v.ConfigFileUsed()); err == nil && !v.InConfig("read_only") {
174+
v.SetDefault("read_only", false)
175+
}
176+
}
177+
178+
// materializeReadOnly pins read_only in every config file this version writes,
179+
// so a freshly created file isn't mistaken for a pre-upgrade install and
180+
// grandfathered to false by MigrateReadOnly on the next load. Must be called
181+
// before the file is rewritten.
182+
func materializeReadOnly(v *viper.Viper) {
183+
if v.IsSet("read_only") {
184+
return
185+
}
186+
if _, err := os.Stat(v.ConfigFileUsed()); err == nil {
187+
v.Set("read_only", false)
188+
} else {
189+
v.Set("read_only", DefaultReadOnly)
190+
}
191+
}
192+
162193
func FromViper(v *viper.Viper) (*Config, error) {
163194
cfg := &Config{
164195
ConfigDir: filepath.Dir(v.ConfigFileUsed()),
@@ -267,6 +298,7 @@ func (c *Config) Set(key, value string) error {
267298
v.ReadInConfig()
268299

269300
v.Set(key, validated)
301+
materializeReadOnly(v)
270302

271303
if err := v.WriteConfigAs(configFile); err != nil {
272304
return fmt.Errorf("error writing config file: %w", err)
@@ -493,6 +525,14 @@ func (c *Config) Unset(key string) error {
493525
return fmt.Errorf("unknown configuration key: %s", key)
494526
}
495527

528+
// Keep read_only materialized (see materializeReadOnly); unsetting it
529+
// restores the current default.
530+
if key == "read_only" {
531+
vNew.Set("read_only", DefaultReadOnly)
532+
} else {
533+
materializeReadOnly(vNew)
534+
}
535+
496536
// Apply the default to the current global viper state
497537
if def, ok := defaultValues[key]; ok {
498538
if _, err := c.UpdateField(key, def); err != nil {
@@ -515,6 +555,8 @@ func (c *Config) Reset() error {
515555
v := viper.New()
516556
v.SetConfigFile(configFile)
517557

558+
v.Set("read_only", DefaultReadOnly)
559+
518560
if err := v.WriteConfigAs(configFile); err != nil {
519561
return fmt.Errorf("error writing config file: %w", err)
520562
}

internal/tiger/config/config_test.go

Lines changed: 153 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"testing"
99

1010
"github.com/spf13/pflag"
11+
"github.com/spf13/viper"
12+
1113
"github.com/timescale/tiger-cli/internal/tiger/util"
1214
)
1315

@@ -70,8 +72,10 @@ func TestLoad_DefaultValues(t *testing.T) {
7072
if cfg.Analytics != DefaultAnalytics {
7173
t.Errorf("Expected Analytics %t, got %t", DefaultAnalytics, cfg.Analytics)
7274
}
73-
if cfg.ReadOnly != DefaultReadOnly {
74-
t.Errorf("Expected ReadOnly %t, got %t", DefaultReadOnly, cfg.ReadOnly)
75+
// setupTestConfig writes a config file without read_only, so it's
76+
// grandfathered to false (see MigrateReadOnly).
77+
if cfg.ReadOnly {
78+
t.Errorf("Expected ReadOnly false for pre-existing config file, got true")
7579
}
7680
if cfg.ConfigDir != tmpDir {
7781
t.Errorf("Expected ConfigDir %s, got %s", tmpDir, cfg.ConfigDir)
@@ -176,6 +180,153 @@ func TestLoad_MigrateVersionCheck(t *testing.T) {
176180
}
177181
}
178182

183+
func TestLoad_MigrateReadOnly(t *testing.T) {
184+
tests := []struct {
185+
name string
186+
fileBody *string // nil means no config file
187+
env map[string]string
188+
want bool
189+
}{
190+
{
191+
name: "no config file gets read-only default",
192+
fileBody: nil,
193+
want: DefaultReadOnly,
194+
},
195+
{
196+
name: "pre-existing config file without key is grandfathered",
197+
fileBody: util.Ptr("output: json\n"),
198+
want: false,
199+
},
200+
{
201+
name: "explicit read_only in config file is respected",
202+
fileBody: util.Ptr("read_only: true\n"),
203+
want: true,
204+
},
205+
{
206+
name: "env var overrides grandfathered value",
207+
fileBody: util.Ptr("output: json\n"),
208+
env: map[string]string{"TIGER_READ_ONLY": "true"},
209+
want: true,
210+
},
211+
}
212+
213+
for _, tt := range tests {
214+
t.Run(tt.name, func(t *testing.T) {
215+
tmpDir := t.TempDir()
216+
t.Cleanup(ResetGlobalConfig)
217+
218+
if tt.fileBody != nil {
219+
configFile := GetConfigFile(tmpDir)
220+
if err := os.WriteFile(configFile, []byte(*tt.fileBody), 0644); err != nil {
221+
t.Fatalf("Failed to write config file: %v", err)
222+
}
223+
}
224+
225+
os.Setenv("TIGER_CONFIG_DIR", tmpDir)
226+
t.Cleanup(func() { os.Unsetenv("TIGER_CONFIG_DIR") })
227+
for k, val := range tt.env {
228+
os.Setenv(k, val)
229+
t.Cleanup(func() { os.Unsetenv(k) })
230+
}
231+
232+
setupViper(t, tmpDir)
233+
234+
cfg, err := Load()
235+
if err != nil {
236+
t.Fatalf("Load() failed: %v", err)
237+
}
238+
if cfg.ReadOnly != tt.want {
239+
t.Errorf("ReadOnly = %t, want %t", cfg.ReadOnly, tt.want)
240+
}
241+
})
242+
}
243+
}
244+
245+
// TestReadOnlyMaterializedOnWrite verifies that Set/Unset/Reset record
246+
// read_only explicitly, so rewritten files aren't grandfathered to false.
247+
func TestReadOnlyMaterializedOnWrite(t *testing.T) {
248+
loadReadOnly := func(t *testing.T, tmpDir string) bool {
249+
t.Helper()
250+
ResetGlobalConfig()
251+
setupViper(t, tmpDir)
252+
cfg, err := Load()
253+
if err != nil {
254+
t.Fatalf("Load() failed: %v", err)
255+
}
256+
return cfg.ReadOnly
257+
}
258+
259+
setup := func(t *testing.T, fileBody *string) (string, *Config) {
260+
t.Helper()
261+
tmpDir := t.TempDir()
262+
t.Cleanup(ResetGlobalConfig)
263+
os.Setenv("TIGER_CONFIG_DIR", tmpDir)
264+
t.Cleanup(func() { os.Unsetenv("TIGER_CONFIG_DIR") })
265+
if fileBody != nil {
266+
if err := os.WriteFile(GetConfigFile(tmpDir), []byte(*fileBody), 0644); err != nil {
267+
t.Fatalf("Failed to write config file: %v", err)
268+
}
269+
}
270+
setupViper(t, tmpDir)
271+
cfg, err := Load()
272+
if err != nil {
273+
t.Fatalf("Load() failed: %v", err)
274+
}
275+
return tmpDir, cfg
276+
}
277+
278+
t.Run("Set on fresh install pins read-only default", func(t *testing.T) {
279+
tmpDir, cfg := setup(t, nil)
280+
if err := cfg.Set("output", "json"); err != nil {
281+
t.Fatalf("Set() failed: %v", err)
282+
}
283+
if got := loadReadOnly(t, tmpDir); got != DefaultReadOnly {
284+
t.Errorf("ReadOnly = %t after Set on fresh install, want %t", got, DefaultReadOnly)
285+
}
286+
})
287+
288+
t.Run("Set on grandfathered config pins false", func(t *testing.T) {
289+
tmpDir, cfg := setup(t, util.Ptr("service_id: svc-123\n"))
290+
if err := cfg.Set("output", "json"); err != nil {
291+
t.Fatalf("Set() failed: %v", err)
292+
}
293+
// Assert on the file itself rather than the loaded value: the
294+
// grandfathering shim would yield an effective false even if Set
295+
// failed to write the key.
296+
v := viper.New()
297+
v.SetConfigFile(GetConfigFile(tmpDir))
298+
if err := v.ReadInConfig(); err != nil {
299+
t.Fatalf("Failed to read config file: %v", err)
300+
}
301+
if !v.InConfig("read_only") {
302+
t.Fatal("read_only not written to config file by Set")
303+
}
304+
if v.GetBool("read_only") {
305+
t.Error("read_only = true in config file after Set on grandfathered config, want false")
306+
}
307+
})
308+
309+
t.Run("Unset read_only restores current default", func(t *testing.T) {
310+
tmpDir, cfg := setup(t, util.Ptr("read_only: false\n"))
311+
if err := cfg.Unset("read_only"); err != nil {
312+
t.Fatalf("Unset() failed: %v", err)
313+
}
314+
if got := loadReadOnly(t, tmpDir); got != DefaultReadOnly {
315+
t.Errorf("ReadOnly = %t after Unset, want %t", got, DefaultReadOnly)
316+
}
317+
})
318+
319+
t.Run("Reset restores current default", func(t *testing.T) {
320+
tmpDir, cfg := setup(t, util.Ptr("read_only: false\noutput: json\n"))
321+
if err := cfg.Reset(); err != nil {
322+
t.Fatalf("Reset() failed: %v", err)
323+
}
324+
if got := loadReadOnly(t, tmpDir); got != DefaultReadOnly {
325+
t.Errorf("ReadOnly = %t after Reset, want %t", got, DefaultReadOnly)
326+
}
327+
})
328+
}
329+
179330
func TestLoad_FromEnvironmentVariables(t *testing.T) {
180331
tmpDir := setupTestConfig(t)
181332

specs/spec.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ All configuration options can be set via `tiger config set <key> <value>`:
4848
- `docs_mcp` - Enable/disable docs MCP proxy (default: true)
4949
- `output` - Output format: json, yaml, or table (default: table)
5050
- `password_storage` - Password storage method: keyring, pgpass, or none (default: keyring)
51-
- `read_only` - When `true`, mutating operations are refused: `tiger service create`/`fork`/`start`/`stop`/`resize`/`update-password`/`delete` and their MCP equivalents return an error, and `tiger db connect`/`connection-string`/`db_execute_query` open against an immutable read-only database connection regardless of `--read-only` (default: false). See `specs/spec_mcp.md` for details.
51+
- `read_only` - When `true`, mutating operations are refused: `tiger service create`/`fork`/`start`/`stop`/`resize`/`update-password`/`delete` and their MCP equivalents return an error, and `tiger db connect`/`connection-string`/`db_execute_query` open against an immutable read-only database connection regardless of `--read-only` (default: true for new installations). See `specs/spec_mcp.md` for details.
5252
- `service_id` - Default service ID
5353
- `version_check` - When true, the CLI checks for a newer version on each invocation (in an interactive terminal) and prints a notice if one is available; false to disable (default: true)
5454

specs/spec_mcp.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ When `read_only` is `true` (or `TIGER_READ_ONLY=true`), Tiger refuses any mutati
6969

7070
Intended for AI agents that should be able to read Tiger Cloud resources without risk of mutation. `tsdb_admin.read_only_connection` is a Tiger Cloud GUC injected as a startup `options` parameter; it activates an immutable read-only connection so writes and DDL are rejected by the server itself and cannot be re-enabled with a `SET` statement.
7171

72-
To toggle: `tiger config set read_only true` / `tiger config unset read_only`.
72+
Read-only mode is enabled by default for new installations. Config files created by older CLI versions don't contain the `read_only` key and are grandfathered to `false` (see `MigrateReadOnly` in `internal/tiger/config/config.go`), so upgrading doesn't break existing workflows. To toggle: `tiger config set read_only false` / `tiger config set read_only true`.
7373

7474
When read-only mode is enabled, the MCP server includes a warning in its `initialize` response `instructions` field listing the gated tools and asking the LLM to inform the user before gathering inputs for them. The instructions are read at server start; if the user toggles `read_only` mid-session, the warning is stale until the MCP client restarts (the gate and the GUC injection are both unaffected because handlers reload config per call).
7575

0 commit comments

Comments
 (0)