diff --git a/internal/command/mcp/config.go b/internal/command/mcp/config.go index 991505e0cf..31e55fda82 100644 --- a/internal/command/mcp/config.go +++ b/internal/command/mcp/config.go @@ -567,8 +567,7 @@ func UpdateConfig(ctx context.Context, path string, configKey string, server str return fmt.Errorf("failed to marshal updated configuration: %w", err) } - err = os.WriteFile(path, updatedData, 0644) - if err != nil { + if err := writeConfigFile(path, updatedData); err != nil { return fmt.Errorf("Failed to write updated configuration to %s: %v", path, err) } @@ -599,6 +598,17 @@ func runRemove(ctx context.Context) error { return nil } +// writeConfigFile writes an MCP client configuration readable only by its +// owner, since the configuration can carry a Fly API token. +func writeConfigFile(path string, data []byte) error { + if err := os.WriteFile(path, data, 0o600); err != nil { + return err + } + // os.WriteFile only applies perm on file creation; Chmod explicitly so + // the mode is applied on rewrite as well. + return os.Chmod(path, 0o600) +} + // removeConfig removes the MCP server from the configuration at the specified path func removeConfig(ctx context.Context, path string, configKey string, name string) error { log := logger.FromContext(ctx) @@ -654,8 +664,7 @@ func removeConfig(ctx context.Context, path string, configKey string, name strin return fmt.Errorf("failed to marshal updated configuration: %w", err) } - err = os.WriteFile(path, updatedData, 0644) - if err != nil { + if err := writeConfigFile(path, updatedData); err != nil { return fmt.Errorf("Failed to write updated configuration to %s: %v", path, err) } diff --git a/internal/command/mcp/config_test.go b/internal/command/mcp/config_test.go index c76d9de4dd..3536a50f45 100644 --- a/internal/command/mcp/config_test.go +++ b/internal/command/mcp/config_test.go @@ -1,7 +1,14 @@ package mcp import ( + "context" + "io" + "os" + "path/filepath" + "runtime" "testing" + + "github.com/superfly/flyctl/internal/logger" ) func TestUnmarshalJSONC(t *testing.T) { @@ -100,3 +107,67 @@ func TestUnmarshalJSONC(t *testing.T) { } }) } + +func TestUpdateConfig_writesConfigAt0600_freshFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("file modes are not meaningful on windows") + } + path := filepath.Join(t.TempDir(), "client", "mcp.json") + + if err := UpdateConfig(context.Background(), path, "", "flyctl", "flyctl", []string{"mcp", "server"}); err != nil { + t.Fatalf("UpdateConfig: %v", err) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat config: %v", err) + } + if got, want := info.Mode().Perm(), os.FileMode(0o600); got != want { + t.Errorf("config mode = %o, want %o", got, want) + } +} + +func TestUpdateConfig_appliesPermsToExistingFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("file modes are not meaningful on windows") + } + path := filepath.Join(t.TempDir(), "mcp.json") + if err := os.WriteFile(path, []byte(`{"mcpServers": {}}`), 0o644); err != nil { + t.Fatalf("seed config: %v", err) + } + + if err := UpdateConfig(context.Background(), path, "", "flyctl", "flyctl", []string{"mcp", "server"}); err != nil { + t.Fatalf("UpdateConfig: %v", err) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat config: %v", err) + } + if got, want := info.Mode().Perm(), os.FileMode(0o600); got != want { + t.Errorf("config mode = %o, want %o", got, want) + } +} + +func TestRemoveConfig_appliesPermsToExistingFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("file modes are not meaningful on windows") + } + path := filepath.Join(t.TempDir(), "mcp.json") + if err := os.WriteFile(path, []byte(`{"mcpServers": {"flyctl": {"command": "flyctl"}}}`), 0o644); err != nil { + t.Fatalf("seed config: %v", err) + } + ctx := logger.NewContext(context.Background(), logger.New(io.Discard, logger.Error, false)) + + if err := removeConfig(ctx, path, "mcpServers", "flyctl"); err != nil { + t.Fatalf("removeConfig: %v", err) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat config: %v", err) + } + if got, want := info.Mode().Perm(), os.FileMode(0o600); got != want { + t.Errorf("config mode = %o, want %o", got, want) + } +}