From dc825f4a2196857198ca65fcf1155c4de7514468 Mon Sep 17 00:00:00 2001 From: Farid Ismailov Date: Mon, 20 Apr 2026 00:28:03 +0300 Subject: [PATCH 1/2] feat(cli): persist token and TLS client cert paths for gRPC Extend profile YAML, dial with optional mTLS and bearer metadata, Unix file mode 0600 when any profile stores a token. Adds configure prompts and tests. Made-with: Cursor --- core/cli/configure.go | 55 ++++++++++++++--- core/client/grpc.go | 103 ++++++++++++++++++++++++++++++-- core/client/grpc_test.go | 52 ++++++++++++++++ core/cmd/data/client.go | 7 +-- core/cmd/permission/client.go | 7 +-- core/cmd/schema/client.go | 7 +-- core/cmd/tenancy/client.go | 7 +-- core/config/config.go | 41 ++++++++++--- core/config/config_perm_test.go | 49 +++++++++++++++ core/config/config_test.go | 19 ++++++ 10 files changed, 312 insertions(+), 35 deletions(-) create mode 100644 core/client/grpc_test.go create mode 100644 core/config/config_perm_test.go create mode 100644 core/config/config_test.go diff --git a/core/cli/configure.go b/core/cli/configure.go index a533aba..e1f0066 100644 --- a/core/cli/configure.go +++ b/core/cli/configure.go @@ -102,12 +102,48 @@ func runE(cmd *cobra.Command, _ []string) error { return err } - resp, err := client.New(url) + token, err := tui.StringPrompt( + "Bearer token (optional, leave empty if none)", + "", + config.CliConfig.Token, + ) + if err != nil { + return err + } + + certPath, err := tui.StringPrompt( + "TLS client certificate path (optional, PEM)", + "", + config.CliConfig.CertPath, + ) + if err != nil { + return err + } + + certKeyPath, err := tui.StringPrompt( + "TLS client private key path (optional, PEM)", + "", + config.CliConfig.CertKeyPath, + ) + if err != nil { + return err + } + + conn := client.Params{ + Endpoint: url, + Token: token, + CertPath: certPath, + CertKeyPath: certKeyPath, + } + resp, err := client.New(conn) + if err != nil { + return fmt.Errorf("connect to permify: %w", err) + } // Todo: Implement pagination tenants, err := resp.Tenancy.List(context.Background(), &v1.TenantListRequest{}) if err != nil { - logger.Log.Fatal(err) + return err } tenantNames := []string{} @@ -117,16 +153,21 @@ func runE(cmd *cobra.Command, _ []string) error { tenantNames = append(tenantNames, nameID) tenantIds[nameID] = tenant.Id } - + if len(tenantNames) == 0 { + return fmt.Errorf("no tenants returned from server; check endpoint and access") + } + tenant, err := tui.Choice("Select a tenant: ", tenantNames) if err != nil { - logger.Log.Error(err) + return err } config.CliConfig.PermifyURL = url config.CliConfig.Tenant = tenantIds[tenant] - err = config.Write() - if err != nil { - logger.Log.Error(err) + config.CliConfig.Token = token + config.CliConfig.CertPath = certPath + config.CliConfig.CertKeyPath = certKeyPath + if err := config.Write(); err != nil { + return err } logger.Log.Info("successfully configured ", "config file", configFile) return nil diff --git a/core/client/grpc.go b/core/client/grpc.go index 11835df..6ca8b42 100644 --- a/core/client/grpc.go +++ b/core/client/grpc.go @@ -2,19 +2,112 @@ package client import ( + "crypto/tls" + "fmt" + "net" + "strings" + + "github.com/Permify/permify-cli/core/config" permify "github.com/Permify/permify-go/v1" "google.golang.org/grpc" + "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" ) -// New initializes a new permify client -func New(endpoint string) (*permify.Client, error) { +type Params struct { + Endpoint string + Token string + CertPath string + CertKeyPath string +} + +func normalizeGRPCEndpoint(endpoint string) string { + e := strings.TrimSpace(endpoint) + lower := strings.ToLower(e) + for _, prefix := range []string{"https://", "http://", "grpc://", "grpcs://"} { + if strings.HasPrefix(lower, prefix) { + return strings.TrimSpace(e[len(prefix):]) + } + } + return e +} + +func stripBearerPrefix(token string) string { + t := strings.TrimSpace(token) + if len(t) >= 7 && strings.EqualFold(t[:7], "bearer ") { + return strings.TrimSpace(t[7:]) + } + return t +} + +func serverNameForDial(target string) string { + host, _, err := net.SplitHostPort(target) + if err != nil { + return target + } + return host +} + +func dialOptions(p Params) ([]grpc.DialOption, bool, error) { + var opts []grpc.DialOption + tlsInUse := false + + switch { + case p.CertPath != "" && p.CertKeyPath != "": + cert, err := tls.LoadX509KeyPair(p.CertPath, p.CertKeyPath) + if err != nil { + return nil, false, fmt.Errorf("load client certificate: %w", err) + } + target := normalizeGRPCEndpoint(p.Endpoint) + tlsCfg := &tls.Config{ + Certificates: []tls.Certificate{cert}, + MinVersion: tls.VersionTLS12, + ServerName: serverNameForDial(target), + } + opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsCfg))) + tlsInUse = true + case p.CertPath != "" || p.CertKeyPath != "": + return nil, false, fmt.Errorf("both cert_path and cert_key_path must be set together for TLS client authentication") + default: + opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials())) + } + + token := stripBearerPrefix(p.Token) + if token != "" { + md := map[string]string{"authorization": "Bearer " + token} + if tlsInUse { + opts = append(opts, grpc.WithPerRPCCredentials(secureTokenCredentials(md))) + } else { + opts = append(opts, grpc.WithPerRPCCredentials(nonSecureTokenCredentials(md))) + } + } + + return opts, tlsInUse, nil +} + +func New(p Params) (*permify.Client, error) { + target := normalizeGRPCEndpoint(p.Endpoint) + if target == "" { + return nil, fmt.Errorf("endpoint is empty") + } + opts, _, err := dialOptions(p) + if err != nil { + return nil, err + } client, err := permify.NewClient( permify.Config{ - Endpoint: endpoint, + Endpoint: target, }, - // Todo: Implement secure call with tls certificate - grpc.WithTransportCredentials(insecure.NewCredentials()), + opts..., ) return client, err } + +func NewFromCLIConfig() (*permify.Client, error) { + return New(Params{ + Endpoint: config.CliConfig.PermifyURL, + Token: config.CliConfig.Token, + CertPath: config.CliConfig.CertPath, + CertKeyPath: config.CliConfig.CertKeyPath, + }) +} diff --git a/core/client/grpc_test.go b/core/client/grpc_test.go new file mode 100644 index 0000000..e02b88d --- /dev/null +++ b/core/client/grpc_test.go @@ -0,0 +1,52 @@ +package client + +import "testing" + +func TestNormalizeGRPCEndpoint(t *testing.T) { + t.Parallel() + cases := []struct { + in, want string + }{ + {"localhost:3478", "localhost:3478"}, + {"https://api.example.com:443", "api.example.com:443"}, + {" grpc://127.0.0.1:9999 ", "127.0.0.1:9999"}, + {"HTTP://HOST:1", "HOST:1"}, + {"", ""}, + } + for _, tc := range cases { + if got := normalizeGRPCEndpoint(tc.in); got != tc.want { + t.Errorf("normalizeGRPCEndpoint(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestStripBearerPrefix(t *testing.T) { + t.Parallel() + cases := []struct { + in, want string + }{ + {"", ""}, + {" abc ", "abc"}, + {"Bearer tok", "tok"}, + {"bearer tok2 ", "tok2"}, + {"BEARER x", "x"}, + {"notbearer x", "notbearer x"}, + } + for _, tc := range cases { + if got := stripBearerPrefix(tc.in); got != tc.want { + t.Errorf("stripBearerPrefix(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestDialOptionsPartialCertPaths(t *testing.T) { + t.Parallel() + _, _, err := dialOptions(Params{ + Endpoint: "localhost:1", + CertPath: "/only/cert.pem", + CertKeyPath: "", + }) + if err == nil { + t.Fatal("expected error when only cert path is set") + } +} diff --git a/core/cmd/data/client.go b/core/cmd/data/client.go index a567b61..c21b876 100644 --- a/core/cmd/data/client.go +++ b/core/cmd/data/client.go @@ -4,16 +4,15 @@ import ( "os" "github.com/Permify/permify-cli/core/client" - "github.com/Permify/permify-cli/core/config" v1 "github.com/Permify/permify-go/generated/base/v1" "github.com/charmbracelet/log" ) func Client() v1.DataClient { - c, err := client.New(config.CliConfig.PermifyURL) + c, err := client.NewFromCLIConfig() if err != nil { log.Error("Error initializing permify client. Check the configuration or rerun `permify configure`") - os.Exit(-1) + os.Exit(-1) } return c.Data -} \ No newline at end of file +} diff --git a/core/cmd/permission/client.go b/core/cmd/permission/client.go index 092f240..59aec68 100644 --- a/core/cmd/permission/client.go +++ b/core/cmd/permission/client.go @@ -4,16 +4,15 @@ import ( "os" "github.com/Permify/permify-cli/core/client" - "github.com/Permify/permify-cli/core/config" v1 "github.com/Permify/permify-go/generated/base/v1" "github.com/charmbracelet/log" ) func Client() v1.PermissionClient { - c, err := client.New(config.CliConfig.PermifyURL) + c, err := client.NewFromCLIConfig() if err != nil { log.Error("Error initializing permify client. Check the configuration or rerun `permify configure`") - os.Exit(-1) + os.Exit(-1) } return c.Permission -} \ No newline at end of file +} diff --git a/core/cmd/schema/client.go b/core/cmd/schema/client.go index 6d0f3c1..befe134 100644 --- a/core/cmd/schema/client.go +++ b/core/cmd/schema/client.go @@ -4,16 +4,15 @@ import ( "os" "github.com/Permify/permify-cli/core/client" - "github.com/Permify/permify-cli/core/config" v1 "github.com/Permify/permify-go/generated/base/v1" "github.com/charmbracelet/log" ) func Client() v1.SchemaClient { - c, err := client.New(config.CliConfig.PermifyURL) + c, err := client.NewFromCLIConfig() if err != nil { log.Error("Error initializing permify client. Check the configuration or rerun `permify configure`") - os.Exit(-1) + os.Exit(-1) } return c.Schema -} \ No newline at end of file +} diff --git a/core/cmd/tenancy/client.go b/core/cmd/tenancy/client.go index 74c8213..0fbf262 100644 --- a/core/cmd/tenancy/client.go +++ b/core/cmd/tenancy/client.go @@ -4,16 +4,15 @@ import ( "os" "github.com/Permify/permify-cli/core/client" - "github.com/Permify/permify-cli/core/config" v1 "github.com/Permify/permify-go/generated/base/v1" "github.com/charmbracelet/log" ) func Client() v1.TenancyClient { - c, err := client.New(config.CliConfig.PermifyURL) + c, err := client.NewFromCLIConfig() if err != nil { log.Error("Error initializing permify client. Check the configuration or rerun `permify configure`") - os.Exit(-1) + os.Exit(-1) } return c.Tenancy -} \ No newline at end of file +} diff --git a/core/config/config.go b/core/config/config.go index c9bdebb..ea533ce 100644 --- a/core/config/config.go +++ b/core/config/config.go @@ -5,6 +5,7 @@ import ( "fmt" "io/fs" "os" + "runtime" "strings" "github.com/Permify/permify-cli/core/logger" @@ -25,9 +26,31 @@ type ProfileConfigs struct { // CoreConfig is the config struct type CoreConfig struct { - PermifyURL string `yaml:"permify_url"` - Tenant string `yaml:"tenant"` - SslEnabled bool `yaml:"-"` + PermifyURL string `yaml:"permify_url"` + Tenant string `yaml:"tenant"` + Token string `yaml:"token,omitempty"` + CertPath string `yaml:"cert_path,omitempty"` + CertKeyPath string `yaml:"cert_key_path,omitempty"` + SslEnabled bool `yaml:"-"` +} + +func anyProfileStoresToken(configs map[string]CoreConfig) bool { + for _, c := range configs { + if strings.TrimSpace(c.Token) != "" { + return true + } + } + return false +} + +func tightenConfigFilePermissions(path string, configs map[string]CoreConfig) error { + if runtime.GOOS == "windows" { + return nil + } + if anyProfileStoresToken(configs) { + return os.Chmod(path, 0o600) + } + return os.Chmod(path, 0o644) } // IsConfigured checks if permctl cli has been configured @@ -84,8 +107,10 @@ func New(file string, profile string) error { if err != nil { return err } - err = os.WriteFile(file, newConfigDataByte, fs.FileMode(0644)) - return err + if err := os.WriteFile(file, newConfigDataByte, fs.FileMode(0o644)); err != nil { + return err + } + return tightenConfigFilePermissions(file, profileConfigs.Configs) } // Write the config file @@ -100,6 +125,8 @@ func Write() error { if err != nil { return err } - err = os.WriteFile(profileConfigs.File, newConfigDataByte, fs.FileMode(0644)) - return err + if err := os.WriteFile(profileConfigs.File, newConfigDataByte, fs.FileMode(0o644)); err != nil { + return err + } + return tightenConfigFilePermissions(profileConfigs.File, profileConfigs.Configs) } diff --git a/core/config/config_perm_test.go b/core/config/config_perm_test.go new file mode 100644 index 0000000..c5e1c37 --- /dev/null +++ b/core/config/config_perm_test.go @@ -0,0 +1,49 @@ +//go:build !windows + +package config + +import ( + "os" + "path/filepath" + "testing" +) + +func TestTightenConfigFilePermissionsWithToken(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "cfg.yaml") + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + cfgs := map[string]CoreConfig{"default": {Token: "secret"}} + if err := tightenConfigFilePermissions(path, cfgs); err != nil { + t.Fatal(err) + } + st, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if m := st.Mode().Perm(); m != 0o600 { + t.Fatalf("want mode 0600, got %04o", m) + } +} + +func TestTightenConfigFilePermissionsNoToken(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "cfg.yaml") + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + cfgs := map[string]CoreConfig{"default": {PermifyURL: "localhost:1", Tenant: "t"}} + if err := tightenConfigFilePermissions(path, cfgs); err != nil { + t.Fatal(err) + } + st, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if m := st.Mode().Perm(); m != 0o644 { + t.Fatalf("want mode 0644, got %04o", m) + } +} diff --git a/core/config/config_test.go b/core/config/config_test.go new file mode 100644 index 0000000..da48b15 --- /dev/null +++ b/core/config/config_test.go @@ -0,0 +1,19 @@ +package config + +import "testing" + +func TestAnyProfileStoresToken(t *testing.T) { + t.Parallel() + if anyProfileStoresToken(map[string]CoreConfig{"a": {}}) { + t.Fatal("expected false for empty token map") + } + if anyProfileStoresToken(map[string]CoreConfig{"a": {Token: " "}}) { + t.Fatal("expected false for whitespace-only token") + } + if !anyProfileStoresToken(map[string]CoreConfig{"a": {Token: "x"}}) { + t.Fatal("expected true when a profile has a token") + } + if !anyProfileStoresToken(map[string]CoreConfig{"a": {}, "b": {Token: "y"}}) { + t.Fatal("expected true when any profile has a token") + } +} From 804d2562b14d4be6f28b0cdd37502eb9dad897ee Mon Sep 17 00:00:00 2001 From: Farid Ismailov Date: Mon, 20 Apr 2026 00:44:27 +0300 Subject: [PATCH 2/2] fix(cli): TLS for bearer tokens; atomic sensitive config write - Treat https:// and grpcs:// as server TLS without client certs; refuse token on plaintext. - Remove nonSecureTokenCredentials bypass of RequireTransportSecurity. - On Unix, write configs that store a token via chmodded temp + rename (no 0644 window). Made-with: Cursor --- core/client/grpc.go | 48 +++++++++++++++++-------- core/client/grpc_test.go | 55 +++++++++++++++++++++++------ core/client/utils.go | 14 -------- core/config/config.go | 62 ++++++++++++++++++++++++++------- core/config/config_perm_test.go | 16 +++------ 5 files changed, 134 insertions(+), 61 deletions(-) diff --git a/core/client/grpc.go b/core/client/grpc.go index 6ca8b42..cdf80db 100644 --- a/core/client/grpc.go +++ b/core/client/grpc.go @@ -21,15 +21,29 @@ type Params struct { CertKeyPath string } -func normalizeGRPCEndpoint(endpoint string) string { +// parsedGRPCEndpoint is the dial target plus whether the URL explicitly required TLS to the server +// (https:// or grpcs://). Stripping the scheme must not silently downgrade to plaintext when a token is used. +type parsedGRPCEndpoint struct { + Target string + ServerTLSScheme bool +} + +func parseGRPCEndpoint(endpoint string) parsedGRPCEndpoint { e := strings.TrimSpace(endpoint) lower := strings.ToLower(e) - for _, prefix := range []string{"https://", "http://", "grpc://", "grpcs://"} { - if strings.HasPrefix(lower, prefix) { - return strings.TrimSpace(e[len(prefix):]) - } + if strings.HasPrefix(lower, "https://") { + return parsedGRPCEndpoint{Target: strings.TrimSpace(e[len("https://"):]), ServerTLSScheme: true} + } + if strings.HasPrefix(lower, "grpcs://") { + return parsedGRPCEndpoint{Target: strings.TrimSpace(e[len("grpcs://"):]), ServerTLSScheme: true} + } + if strings.HasPrefix(lower, "http://") { + return parsedGRPCEndpoint{Target: strings.TrimSpace(e[len("http://"):]), ServerTLSScheme: false} + } + if strings.HasPrefix(lower, "grpc://") { + return parsedGRPCEndpoint{Target: strings.TrimSpace(e[len("grpc://"):]), ServerTLSScheme: false} } - return e + return parsedGRPCEndpoint{Target: e, ServerTLSScheme: false} } func stripBearerPrefix(token string) string { @@ -49,6 +63,7 @@ func serverNameForDial(target string) string { } func dialOptions(p Params) ([]grpc.DialOption, bool, error) { + parsed := parseGRPCEndpoint(p.Endpoint) var opts []grpc.DialOption tlsInUse := false @@ -58,35 +73,40 @@ func dialOptions(p Params) ([]grpc.DialOption, bool, error) { if err != nil { return nil, false, fmt.Errorf("load client certificate: %w", err) } - target := normalizeGRPCEndpoint(p.Endpoint) tlsCfg := &tls.Config{ Certificates: []tls.Certificate{cert}, MinVersion: tls.VersionTLS12, - ServerName: serverNameForDial(target), + ServerName: serverNameForDial(parsed.Target), } opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsCfg))) tlsInUse = true case p.CertPath != "" || p.CertKeyPath != "": return nil, false, fmt.Errorf("both cert_path and cert_key_path must be set together for TLS client authentication") + case parsed.ServerTLSScheme: + tlsCfg := &tls.Config{ + MinVersion: tls.VersionTLS12, + ServerName: serverNameForDial(parsed.Target), + } + opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsCfg))) + tlsInUse = true default: opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials())) } token := stripBearerPrefix(p.Token) if token != "" { - md := map[string]string{"authorization": "Bearer " + token} - if tlsInUse { - opts = append(opts, grpc.WithPerRPCCredentials(secureTokenCredentials(md))) - } else { - opts = append(opts, grpc.WithPerRPCCredentials(nonSecureTokenCredentials(md))) + if !tlsInUse { + return nil, false, fmt.Errorf("bearer token requires TLS: use https:// or grpcs:// in permify_url, or set cert_path and cert_key_path for mTLS") } + md := map[string]string{"authorization": "Bearer " + token} + opts = append(opts, grpc.WithPerRPCCredentials(secureTokenCredentials(md))) } return opts, tlsInUse, nil } func New(p Params) (*permify.Client, error) { - target := normalizeGRPCEndpoint(p.Endpoint) + target := parseGRPCEndpoint(p.Endpoint).Target if target == "" { return nil, fmt.Errorf("endpoint is empty") } diff --git a/core/client/grpc_test.go b/core/client/grpc_test.go index e02b88d..cdeb474 100644 --- a/core/client/grpc_test.go +++ b/core/client/grpc_test.go @@ -1,21 +1,28 @@ package client -import "testing" +import ( + "strings" + "testing" +) -func TestNormalizeGRPCEndpoint(t *testing.T) { +func TestParseGRPCEndpoint(t *testing.T) { t.Parallel() cases := []struct { - in, want string + in string + wantTarget string + wantServerTLS bool }{ - {"localhost:3478", "localhost:3478"}, - {"https://api.example.com:443", "api.example.com:443"}, - {" grpc://127.0.0.1:9999 ", "127.0.0.1:9999"}, - {"HTTP://HOST:1", "HOST:1"}, - {"", ""}, + {"localhost:3478", "localhost:3478", false}, + {"https://api.example.com:443", "api.example.com:443", true}, + {"grpcs://h:1", "h:1", true}, + {" grpc://127.0.0.1:9999 ", "127.0.0.1:9999", false}, + {"HTTP://HOST:1", "HOST:1", false}, + {"", "", false}, } for _, tc := range cases { - if got := normalizeGRPCEndpoint(tc.in); got != tc.want { - t.Errorf("normalizeGRPCEndpoint(%q) = %q, want %q", tc.in, got, tc.want) + got := parseGRPCEndpoint(tc.in) + if got.Target != tc.wantTarget || got.ServerTLSScheme != tc.wantServerTLS { + t.Errorf("parseGRPCEndpoint(%q) = %#v, want target %q serverTLS %v", tc.in, got, tc.wantTarget, tc.wantServerTLS) } } } @@ -50,3 +57,31 @@ func TestDialOptionsPartialCertPaths(t *testing.T) { t.Fatal("expected error when only cert path is set") } } + +func TestDialOptionsTokenRequiresTLS(t *testing.T) { + t.Parallel() + _, _, err := dialOptions(Params{ + Endpoint: "localhost:3478", + Token: "sekrit", + }) + if err == nil || !strings.Contains(err.Error(), "bearer token requires TLS") { + t.Fatalf("expected TLS requirement error, got %v", err) + } +} + +func TestDialOptionsTokenWithHTTPSScheme(t *testing.T) { + t.Parallel() + opts, tlsInUse, err := dialOptions(Params{ + Endpoint: "https://api.example.com:443", + Token: "tok", + }) + if err != nil { + t.Fatal(err) + } + if !tlsInUse { + t.Fatal("expected TLS in use") + } + if len(opts) < 2 { + t.Fatalf("expected transport + PerRPC options, got %d", len(opts)) + } +} diff --git a/core/client/utils.go b/core/client/utils.go index 918fb7a..b35988d 100644 --- a/core/client/utils.go +++ b/core/client/utils.go @@ -17,17 +17,3 @@ func (c secureTokenCredentials) RequireTransportSecurity() bool { func (c secureTokenCredentials) GetRequestMetadata(context.Context, ...string) (map[string]string, error) { return c, nil // Returns the secure tokens as metadata with no error. } - -// nonSecureTokenCredentials represents a map used for storing non-secure tokens. -// These tokens do not require transport security. -type nonSecureTokenCredentials map[string]string - -// RequireTransportSecurity indicates that transport security is not required for these credentials. -func (c nonSecureTokenCredentials) RequireTransportSecurity() bool { - return false // Transport security is not required for non-secure tokens. -} - -// GetRequestMetadata retrieves the current metadata (non-secure tokens) for a request. -func (c nonSecureTokenCredentials) GetRequestMetadata(_ context.Context, _ ...string) (map[string]string, error) { - return c, nil // Returns the non-secure tokens as metadata with no error. -} diff --git a/core/config/config.go b/core/config/config.go index ea533ce..e471623 100644 --- a/core/config/config.go +++ b/core/config/config.go @@ -5,6 +5,7 @@ import ( "fmt" "io/fs" "os" + "path/filepath" "runtime" "strings" @@ -43,16 +44,59 @@ func anyProfileStoresToken(configs map[string]CoreConfig) bool { return false } -func tightenConfigFilePermissions(path string, configs map[string]CoreConfig) error { +// writeConfigFile writes YAML bytes and sets permissions without leaving a token-bearing file +// world-readable between truncate and chmod (Unix: temp file + rename with mode 0600). +func writeConfigFile(path string, data []byte, configs map[string]CoreConfig) error { + sensitive := anyProfileStoresToken(configs) if runtime.GOOS == "windows" { - return nil + perm := fs.FileMode(0o644) + if sensitive { + perm = 0o666 + } + return os.WriteFile(path, data, perm) + } + if sensitive { + return atomicWriteFileUnix(path, data, 0o600) } - if anyProfileStoresToken(configs) { - return os.Chmod(path, 0o600) + if err := os.WriteFile(path, data, 0o644); err != nil { + return err } return os.Chmod(path, 0o644) } +func atomicWriteFileUnix(path string, data []byte, perm fs.FileMode) error { + dir := filepath.Dir(path) + f, err := os.CreateTemp(dir, ".permctl-config-*.tmp") + if err != nil { + return fmt.Errorf("create temp config: %w", err) + } + tmpPath := f.Name() + cleanup := true + defer func() { + _ = f.Close() + if cleanup { + _ = os.Remove(tmpPath) + } + }() + if err := f.Chmod(perm); err != nil { + return fmt.Errorf("chmod temp config: %w", err) + } + if _, err := f.Write(data); err != nil { + return fmt.Errorf("write temp config: %w", err) + } + if err := f.Sync(); err != nil { + return fmt.Errorf("sync temp config: %w", err) + } + if err := f.Close(); err != nil { + return fmt.Errorf("close temp config: %w", err) + } + if err := os.Rename(tmpPath, path); err != nil { + return fmt.Errorf("replace config: %w", err) + } + cleanup = false + return nil +} + // IsConfigured checks if permctl cli has been configured func IsConfigured(file string, profile string) error { _, err := os.Stat(file) @@ -107,10 +151,7 @@ func New(file string, profile string) error { if err != nil { return err } - if err := os.WriteFile(file, newConfigDataByte, fs.FileMode(0o644)); err != nil { - return err - } - return tightenConfigFilePermissions(file, profileConfigs.Configs) + return writeConfigFile(file, newConfigDataByte, profileConfigs.Configs) } // Write the config file @@ -125,8 +166,5 @@ func Write() error { if err != nil { return err } - if err := os.WriteFile(profileConfigs.File, newConfigDataByte, fs.FileMode(0o644)); err != nil { - return err - } - return tightenConfigFilePermissions(profileConfigs.File, profileConfigs.Configs) + return writeConfigFile(profileConfigs.File, newConfigDataByte, profileConfigs.Configs) } diff --git a/core/config/config_perm_test.go b/core/config/config_perm_test.go index c5e1c37..3bbdf6e 100644 --- a/core/config/config_perm_test.go +++ b/core/config/config_perm_test.go @@ -8,15 +8,12 @@ import ( "testing" ) -func TestTightenConfigFilePermissionsWithToken(t *testing.T) { +func TestWriteConfigFilePermissionsWithToken(t *testing.T) { t.Parallel() dir := t.TempDir() path := filepath.Join(dir, "cfg.yaml") - if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { - t.Fatal(err) - } - cfgs := map[string]CoreConfig{"default": {Token: "secret"}} - if err := tightenConfigFilePermissions(path, cfgs); err != nil { + cfgs := map[string]CoreConfig{"default": {Token: "secret", PermifyURL: "https://x", Tenant: "t"}} + if err := writeConfigFile(path, []byte("k: v\n"), cfgs); err != nil { t.Fatal(err) } st, err := os.Stat(path) @@ -28,15 +25,12 @@ func TestTightenConfigFilePermissionsWithToken(t *testing.T) { } } -func TestTightenConfigFilePermissionsNoToken(t *testing.T) { +func TestWriteConfigFilePermissionsNoToken(t *testing.T) { t.Parallel() dir := t.TempDir() path := filepath.Join(dir, "cfg.yaml") - if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { - t.Fatal(err) - } cfgs := map[string]CoreConfig{"default": {PermifyURL: "localhost:1", Tenant: "t"}} - if err := tightenConfigFilePermissions(path, cfgs); err != nil { + if err := writeConfigFile(path, []byte("k: v\n"), cfgs); err != nil { t.Fatal(err) } st, err := os.Stat(path)