Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions cmd/permctl/permctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,24 @@
package main

import (
"fmt"
"os"
"path/filepath"

"github.com/Permify/permify-cli/core/cli"
)

func main() {
home := os.Getenv("HOME")
defaultConfig := fmt.Sprintf("%s/.permctl", home)
if home == "" {
// os.UserHomeDir covers Windows USERPROFILE and other platforms.
if derived, err := os.UserHomeDir(); err == nil {
home = derived
}
}
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both the HOME environment variable is empty and os.UserHomeDir returns an error, the home variable will remain empty, causing filepath.Join to produce an invalid path like "/.permctl". Consider handling this case explicitly with an error or fallback.

Suggested change
}
}
if home == "" {
// Fallback to the current working directory if home cannot be determined.
if cwd, err := os.Getwd(); err == nil {
home = cwd
} else {
os.Stderr.WriteString("permctl: unable to determine configuration directory (HOME and working directory unavailable)\n")
os.Exit(1)
}
}

Copilot uses AI. Check for mistakes.

defaultConfig := filepath.Join(home, ".permctl")
shortDescription := "permctl is a cli for managing and communicating with permify"
permctl := cli.New("permctl", shortDescription, defaultConfig)
permctl := cli.New("permctl", shortDescription, defaultConfig)
cli.AddComponents(permctl.Cmd)
permctl.Execute()
}
46 changes: 44 additions & 2 deletions core/cli/configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,54 @@ func validateFlags(cmd *cobra.Command, args []string) error {
func runE(cmd *cobra.Command, _ []string) error {
configFile, _ := cmd.Flags().GetString("config")

url, err := tui.StringPrompt("enter permify url", "", config.CliConfig.PermifyURL)
storedCreds, err := client.LoadCredentials()
if err != nil {
return err
}

defaultURL := config.CliConfig.PermifyURL
if defaultURL == "" {
defaultURL = storedCreds.Endpoint
}

url, err := tui.StringPrompt("enter permify url", "", defaultURL)
if err != nil {
return err
}

token, err := tui.StringPrompt("enter api token (optional)", "", storedCreds.Token)
if err != nil {
return err
}

certPath, err := tui.StringPrompt("enter client certificate path (optional)", "", storedCreds.CertPath)
if err != nil {
return err
}

certKey, err := tui.StringPrompt("enter client certificate key path (optional)", "", storedCreds.CertKey)
if err != nil {
return err
}

if (certPath == "") != (certKey == "") {
return fmt.Errorf("both certificate path and key must be provided or omitted")
}

Comment on lines +130 to +133
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XOR comparison using != is correct for checking that both certPath and certKey are either set or unset. However, this duplicates the validation logic already present in buildTransportCredentials (lines 70-72 in grpc.go). Consider removing this duplication or extracting it into a shared validation function.

Suggested change
if (certPath == "") != (certKey == "") {
return fmt.Errorf("both certificate path and key must be provided or omitted")
}

Copilot uses AI. Check for mistakes.
storeErr := client.StoreCredentials(client.Credentials{
Endpoint: url,
Token: token,
CertPath: certPath,
CertKey: certKey,
})
if storeErr != nil {
logger.Log.Error(storeErr)
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stored credentials that fail to save only log an error but don't stop execution. This could lead to users thinking their credentials were saved when they weren't. Consider returning the error to the user, or at minimum, displaying a clear warning message that credentials were not persisted.

Suggested change
logger.Log.Error(storeErr)
logger.Log.Error(storeErr)
return fmt.Errorf("failed to store credentials: %w", storeErr)

Copilot uses AI. Check for mistakes.
}

resp, err := client.New(url)
if err != nil {
return err
}

// Todo: Implement pagination
tenants, err := resp.Tenancy.List(context.Background(), &v1.TenantListRequest{})
Expand All @@ -117,7 +159,7 @@ func runE(cmd *cobra.Command, _ []string) error {
tenantNames = append(tenantNames, nameID)
tenantIds[nameID] = tenant.Id
}

tenant, err := tui.Choice("Select a tenant: ", tenantNames)
if err != nil {
logger.Log.Error(err)
Expand Down
74 changes: 74 additions & 0 deletions core/client/credentials_storage.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package client

import (
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"

"gopkg.in/yaml.v3"
)

// Credentials represents persisted client configuration for the Permify CLI.
type Credentials struct {
Endpoint string `yaml:"endpoint"`
Token string `yaml:"token"`
CertPath string `yaml:"cert_path"`
CertKey string `yaml:"cert_key"`
}

// credentialsFilePath resolves the OS-specific location for the credentials file.
func credentialsFilePath() (string, error) {
home, err := os.UserHomeDir()
if err != nil {
return "", fmt.Errorf("resolve user home directory: %w", err)
}
return filepath.Join(home, ".permify", "credentials"), nil
}

// LoadCredentials fetches stored credentials from disk, returning an empty struct when none exist.
func LoadCredentials() (Credentials, error) {
file, err := credentialsFilePath()
if err != nil {
return Credentials{}, err
}

data, err := os.ReadFile(file)
if errors.Is(err, fs.ErrNotExist) {
return Credentials{}, nil
}
if err != nil {
return Credentials{}, fmt.Errorf("read credentials file %s: %w", file, err)
}

var creds Credentials
if err := yaml.Unmarshal(data, &creds); err != nil {
return Credentials{}, fmt.Errorf("parse credentials file %s: %w", file, err)
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function currently returns an empty Credentials struct when the file doesn't exist, but LoadCredentials is also called in the New function where an error during loading would fail the entire client creation. If there's a corrupted credentials file, this could prevent users from using the CLI even with explicit endpoint parameters. Consider whether LoadCredentials should be more lenient in the New function when an explicit endpoint is provided.

Suggested change
return Credentials{}, fmt.Errorf("parse credentials file %s: %w", file, err)
// Treat corrupted credentials as if no credentials are stored, so callers
// (e.g. client constructors with explicit endpoint params) are not blocked.
return Credentials{}, nil

Copilot uses AI. Check for mistakes.
}

return creds, nil
}

// StoreCredentials writes the provided credentials to disk with user-only permissions.
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "user-only permissions" but doesn't document the specific permission values (0700 for directory, 0600 for file) or explain the security rationale. Consider adding a brief note about why these restrictive permissions are necessary for credential storage.

Suggested change
// StoreCredentials writes the provided credentials to disk with user-only permissions.
// StoreCredentials writes the provided credentials to disk with user-only permissions:
// the credentials directory is created with 0700 and the file with 0600. These restrictive
// modes ensure that only the current user can read or modify the stored credentials, preventing
// other local users from accessing sensitive endpoint tokens or certificates.

Copilot uses AI. Check for mistakes.
func StoreCredentials(creds Credentials) error {
file, err := credentialsFilePath()
if err != nil {
return err
}

if err := os.MkdirAll(filepath.Dir(file), 0700); err != nil {
return fmt.Errorf("create credentials directory: %w", err)
}

data, err := yaml.Marshal(creds)
if err != nil {
return fmt.Errorf("marshal credentials: %w", err)
}

if err := os.WriteFile(file, data, 0600); err != nil {
return fmt.Errorf("write credentials file %s: %w", file, err)
}

return nil
}
80 changes: 76 additions & 4 deletions core/client/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,91 @@
package client

import (
"crypto/tls"
"errors"
"fmt"
"strings"

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
// New initializes a new permify client with optional persisted credentials.
func New(endpoint string) (*permify.Client, error) {
storedCreds, err := LoadCredentials()
if err != nil {
return nil, err
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When LoadCredentials returns an error (line 18-20), the function immediately fails. However, if a user is explicitly providing an endpoint parameter, they should be able to use the client even if stored credentials are corrupted or inaccessible. Consider only treating LoadCredentials errors as fatal when both the endpoint parameter is empty AND credentials cannot be loaded.

Suggested change
return nil, err
// If no endpoint is provided and credentials cannot be loaded, fail.
if endpoint == "" {
return nil, err
}
// If an endpoint is explicitly provided, proceed with empty credentials.
storedCreds = Credentials{}

Copilot uses AI. Check for mistakes.
}

finalEndpoint := endpoint
if finalEndpoint == "" {
finalEndpoint = storedCreds.Endpoint
}
if finalEndpoint == "" {
return nil, errors.New("permify endpoint missing: provide an endpoint or store one in ~/.permify/credentials")
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message hardcodes the path as "~/.permify/credentials", but on Windows this path might not be accurate. Consider making the error message more generic or dynamically including the actual resolved path.

Suggested change
return nil, errors.New("permify endpoint missing: provide an endpoint or store one in ~/.permify/credentials")
return nil, errors.New("permify endpoint missing: provide an endpoint or configure stored credentials")

Copilot uses AI. Check for mistakes.
}

Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic determines TLS usage based on endpoint prefix OR certificate presence. However, if a user provides certificate paths but uses an insecure endpoint (e.g., "grpc://"), the certificates will be loaded but might not be properly utilized due to conflicting security settings. Consider validating this combination or clarifying the precedence.

Suggested change
// Validate conflicting configuration: insecure endpoint scheme with TLS certificates.
lowerEndpoint := strings.ToLower(finalEndpoint)
if (strings.HasPrefix(lowerEndpoint, "grpc://") || strings.HasPrefix(lowerEndpoint, "http://")) &&
(storedCreds.CertPath != "" || storedCreds.CertKey != "") {
return nil, errors.New("insecure endpoint scheme (grpc:// or http://) cannot be used with TLS certificates; use grpcs:// or https:// or remove certificate configuration")
}

Copilot uses AI. Check for mistakes.
useTLS := isSecureEndpoint(finalEndpoint) || storedCreds.CertPath != "" || storedCreds.CertKey != ""

dialOptions, err := buildDialOptions(storedCreds, useTLS)
if err != nil {
return nil, err
}

client, err := permify.NewClient(
permify.Config{
Endpoint: endpoint,
Endpoint: finalEndpoint,
},
// Todo: Implement secure call with tls certificate
grpc.WithTransportCredentials(insecure.NewCredentials()),
dialOptions...,
)
return client, err
}

func buildDialOptions(creds Credentials, useTLS bool) ([]grpc.DialOption, error) {
options := []grpc.DialOption{}
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The options slice is initialized as an empty slice using []grpc.DialOption{}, but this can be simplified to var options []grpc.DialOption or just []grpc.DialOption{} without the type annotation, as it's redundant given the function's return type.

Suggested change
options := []grpc.DialOption{}
var options []grpc.DialOption

Copilot uses AI. Check for mistakes.

transportCredentials, err := buildTransportCredentials(creds, useTLS)
if err != nil {
return nil, err
}
options = append(options, grpc.WithTransportCredentials(transportCredentials))

if creds.Token != "" {
bearer := fmt.Sprintf("Bearer %s", creds.Token)
if transportCredentials.Info().SecurityProtocol != "insecure" {
options = append(options, grpc.WithPerRPCCredentials(secureTokenCredentials{"authorization": bearer}))
} else {
options = append(options, grpc.WithPerRPCCredentials(nonSecureTokenCredentials{"authorization": bearer}))
}
Comment on lines +57 to +62
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sending bearer tokens over insecure connections exposes credentials to interception. Consider either requiring TLS when a token is provided, or at minimum, logging a security warning to alert users of the risk.

Suggested change
bearer := fmt.Sprintf("Bearer %s", creds.Token)
if transportCredentials.Info().SecurityProtocol != "insecure" {
options = append(options, grpc.WithPerRPCCredentials(secureTokenCredentials{"authorization": bearer}))
} else {
options = append(options, grpc.WithPerRPCCredentials(nonSecureTokenCredentials{"authorization": bearer}))
}
if transportCredentials.Info().SecurityProtocol == "insecure" {
return nil, errors.New("cannot use bearer token over insecure connection; enable TLS or omit the token")
}
bearer := fmt.Sprintf("Bearer %s", creds.Token)
options = append(options, grpc.WithPerRPCCredentials(secureTokenCredentials{"authorization": bearer}))

Copilot uses AI. Check for mistakes.
}

return options, nil
}

func buildTransportCredentials(creds Credentials, useTLS bool) (credentials.TransportCredentials, error) {
if creds.CertPath != "" || creds.CertKey != "" {
if creds.CertPath == "" || creds.CertKey == "" {
return nil, errors.New("both certificate path and certificate key must be provided")
}

certificate, err := tls.LoadX509KeyPair(creds.CertPath, creds.CertKey)
if err != nil {
return nil, fmt.Errorf("load tls key pair: %w", err)
Comment on lines +74 to +76
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certificate and key file paths are not validated for existence or accessibility before attempting to load them. This could lead to confusing error messages when tls.LoadX509KeyPair fails. Consider adding explicit file existence checks with clearer error messages.

Copilot uses AI. Check for mistakes.
}

return credentials.NewTLS(&tls.Config{Certificates: []tls.Certificate{certificate}}), nil
}

if useTLS {
return credentials.NewTLS(&tls.Config{}), nil
Comment on lines +79 to +83
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TLS configuration should include MinVersion set to at least TLS 1.2 to prevent use of older, insecure TLS versions. Using an empty tls.Config allows negotiation down to potentially vulnerable TLS 1.0 or 1.1.

Suggested change
return credentials.NewTLS(&tls.Config{Certificates: []tls.Certificate{certificate}}), nil
}
if useTLS {
return credentials.NewTLS(&tls.Config{}), nil
return credentials.NewTLS(&tls.Config{
Certificates: []tls.Certificate{certificate},
MinVersion: tls.VersionTLS12,
}), nil
}
if useTLS {
return credentials.NewTLS(&tls.Config{
MinVersion: tls.VersionTLS12,
}), nil

Copilot uses AI. Check for mistakes.
Comment on lines +79 to +83
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TLS configuration should include MinVersion set to at least TLS 1.2 to prevent use of older, insecure TLS versions. Additionally, when loading client certificates, the configuration should still verify the server certificate by default to prevent man-in-the-middle attacks.

Suggested change
return credentials.NewTLS(&tls.Config{Certificates: []tls.Certificate{certificate}}), nil
}
if useTLS {
return credentials.NewTLS(&tls.Config{}), nil
return credentials.NewTLS(&tls.Config{
Certificates: []tls.Certificate{certificate},
MinVersion: tls.VersionTLS12,
}), nil
}
if useTLS {
return credentials.NewTLS(&tls.Config{
MinVersion: tls.VersionTLS12,
}), nil

Copilot uses AI. Check for mistakes.
}

return insecure.NewCredentials(), nil
}

func isSecureEndpoint(endpoint string) bool {
value := strings.ToLower(endpoint)
return strings.HasPrefix(value, "https://") || strings.HasPrefix(value, "grpcs://")
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TLS detection logic is incomplete. An endpoint with "https://" typically indicates an HTTP API, not a gRPC endpoint. For gRPC, the scheme should be something like "grpc://" or "grpcs://". Using "https://" to infer gRPC TLS could lead to incorrect protocol handling.

Suggested change
return strings.HasPrefix(value, "https://") || strings.HasPrefix(value, "grpcs://")
return strings.HasPrefix(value, "grpcs://")

Copilot uses AI. Check for mistakes.
}