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
34 changes: 34 additions & 0 deletions cmd/permify/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package main

import (
"os"

"github.com/cespare/xxhash/v2"
"github.com/sercand/kuberesolver/v5"
"google.golang.org/grpc/balancer"

consistentbalancer "github.com/Permify/permify/pkg/balancer"
"github.com/Permify/permify/pkg/cmd"
)

func main() {
kuberesolver.RegisterInCluster()
balancer.Register(consistentbalancer.NewBuilder(xxhash.Sum64))

root := cmd.NewRootCommand()
root.AddCommand(cmd.NewServeCommand())
root.AddCommand(cmd.NewValidateCommand())
root.AddCommand(cmd.NewCoverageCommand())
root.AddCommand(cmd.NewGenerateAstCommand())
root.AddCommand(cmd.NewMigrateCommand())
root.AddCommand(cmd.NewVersionCommand())
root.AddCommand(cmd.NewConfigCommand())
root.AddCommand(cmd.NewRepairCommand())

// Add configure command
root.AddCommand(cmd.NewConfigureCommand())
Comment on lines +27 to +29
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix formatting per gofumpt.

Static analysis indicates a formatting issue at line 27 (likely the trailing whitespace before the comment). Run gofumpt -w cmd/permify/main.go to fix.

✏️ Proposed fix
 	root.AddCommand(cmd.NewRepairCommand())
-	
-	// Add configure command
+
+	// Add configure command
 	root.AddCommand(cmd.NewConfigureCommand())
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Add configure command
root.AddCommand(cmd.NewConfigureCommand())
root.AddCommand(cmd.NewRepairCommand())
// Add configure command
root.AddCommand(cmd.NewConfigureCommand())
🧰 Tools
🪛 golangci-lint (2.5.0)

[error] 27-27: File is not properly formatted

(gofumpt)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/permify/main.go` around lines 27 - 29, There is a gofumpt formatting
issue in main.go around the root.AddCommand(cmd.NewConfigureCommand())
comment—remove the trailing whitespace before the comment or reformat the file;
run gofumpt -w cmd/permify/main.go (or gofmt/gofumpt on the file) so the
whitespace/comment formatting around root.AddCommand and
cmd.NewConfigureCommand() is corrected.


if err := root.Execute(); err != nil {
os.Exit(1)
}
}
57 changes: 0 additions & 57 deletions cmd/permify/permify.go

This file was deleted.

Binary file added permify
Binary file not shown.
139 changes: 139 additions & 0 deletions pkg/cmd/configure.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package cmd

import (
"bufio"
"context"
"crypto/tls"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/spf13/cobra"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"
"gopkg.in/yaml.v3"
)

type Credentials struct {
Endpoint string `yaml:"endpoint"`
APIToken string `yaml:"api_token"`
CertPath string `yaml:"cert_path"`
CertKey string `yaml:"cert_key"`
}

func getCredentialsPath() string {
home, err := os.UserHomeDir()
if err != nil {
fmt.Println("Error getting home directory:", err)
os.Exit(1)
}
return filepath.Join(home, ".permify", "credentials")
}

func NewConfigureCommand() *cobra.Command {
return &cobra.Command{
Use: "configure",
Short: "Configure Permify CLI credentials",
Run: func(cmd *cobra.Command, args []string) {
reader := bufio.NewReader(os.Stdin)

fmt.Print("Endpoint (e.g., localhost:3478): ")
endpoint, _ := reader.ReadString('\n')

fmt.Print("API Token: ")
apiToken, _ := reader.ReadString('\n')

fmt.Print("Cert Path (optional): ")
certPath, _ := reader.ReadString('\n')

fmt.Print("Cert Key (optional): ")
certKey, _ := reader.ReadString('\n')
Comment on lines +42 to +52
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Handle errors from ReadString to avoid silent failures.

The errors from reader.ReadString('\n') are silently ignored. If stdin encounters an I/O error or EOF unexpectedly, the user won't be notified and may end up with empty/corrupted credentials.

🛡️ Proposed fix to handle read errors
 			fmt.Print("Endpoint (e.g., localhost:3478): ")
-			endpoint, _ := reader.ReadString('\n')
+			endpoint, err := reader.ReadString('\n')
+			if err != nil {
+				fmt.Println("Error reading endpoint:", err)
+				os.Exit(1)
+			}

 			fmt.Print("API Token: ")
-			apiToken, _ := reader.ReadString('\n')
+			apiToken, err := reader.ReadString('\n')
+			if err != nil {
+				fmt.Println("Error reading API token:", err)
+				os.Exit(1)
+			}

 			fmt.Print("Cert Path (optional): ")
-			certPath, _ := reader.ReadString('\n')
+			certPath, err := reader.ReadString('\n')
+			if err != nil {
+				fmt.Println("Error reading cert path:", err)
+				os.Exit(1)
+			}

 			fmt.Print("Cert Key (optional): ")
-			certKey, _ := reader.ReadString('\n')
+			certKey, err := reader.ReadString('\n')
+			if err != nil {
+				fmt.Println("Error reading cert key:", err)
+				os.Exit(1)
+			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fmt.Print("Endpoint (e.g., localhost:3478): ")
endpoint, _ := reader.ReadString('\n')
fmt.Print("API Token: ")
apiToken, _ := reader.ReadString('\n')
fmt.Print("Cert Path (optional): ")
certPath, _ := reader.ReadString('\n')
fmt.Print("Cert Key (optional): ")
certKey, _ := reader.ReadString('\n')
fmt.Print("Endpoint (e.g., localhost:3478): ")
endpoint, err := reader.ReadString('\n')
if err != nil {
fmt.Println("Error reading endpoint:", err)
os.Exit(1)
}
fmt.Print("API Token: ")
apiToken, err := reader.ReadString('\n')
if err != nil {
fmt.Println("Error reading API token:", err)
os.Exit(1)
}
fmt.Print("Cert Path (optional): ")
certPath, err := reader.ReadString('\n')
if err != nil {
fmt.Println("Error reading cert path:", err)
os.Exit(1)
}
fmt.Print("Cert Key (optional): ")
certKey, err := reader.ReadString('\n')
if err != nil {
fmt.Println("Error reading cert key:", err)
os.Exit(1)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cmd/configure.go` around lines 42 - 52, The calls to
reader.ReadString('\n') in configure.go ignore returned errors and can produce
empty/invalid inputs; update each read (for endpoint, apiToken, certPath,
certKey) to capture the (string, error) result, check the error, and handle it
(e.g., print a clear error via fmt.Fprintln(os.Stderr, ...) and return or
os.Exit(1)). Specifically modify the blocks where endpoint := reader.ReadString,
apiToken := reader.ReadString, certPath := reader.ReadString, and certKey :=
reader.ReadString to use endpoint, err := reader.ReadString('\n') (and likewise
for the others), then if err != nil handle/report and stop further processing.


creds := Credentials{
Endpoint: strings.TrimSpace(endpoint),
APIToken: strings.TrimSpace(apiToken),
CertPath: strings.TrimSpace(certPath),
CertKey: strings.TrimSpace(certKey),
}

data, err := yaml.Marshal(&creds)
if err != nil {
fmt.Println("Error marshaling credentials:", err)
os.Exit(1)
}

credsPath := getCredentialsPath()
credsDir := filepath.Dir(credsPath)

if err := os.MkdirAll(credsDir, 0700); err != nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix formatting per gofumpt.

Static analysis indicates this line has a formatting issue. Run gofumpt -w pkg/cmd/configure.go to fix.

🧰 Tools
🪛 golangci-lint (2.5.0)

[error] 70-70: File is not properly formatted

(gofumpt)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cmd/configure.go` at line 70, The call os.MkdirAll(credsDir, 0700) is
flagged by gofumpt formatting; update the octal permission literal to the modern
Go form or run the formatter. Replace 0700 with 0o700 (or run gofumpt -w
pkg/cmd/configure.go) so the os.MkdirAll(credsDir, 0o700) invocation matches
gofumpt rules.

fmt.Println("Error creating credentials directory:", err)
os.Exit(1)
}

if err := os.WriteFile(credsPath, data, 0600); err != nil {
fmt.Println("Error writing credentials file:", err)
os.Exit(1)
}

fmt.Println("Credentials saved successfully in ~/.permify/credentials")
},
}
}

type TokenAuth struct {
Token string
}

func (t TokenAuth) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) {
return map[string]string{
"authorization": "Bearer " + t.Token,
}, nil
}

// FIX: Blindamos el envío del token (Security Risk)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use English for code comments.

This comment is in Spanish. For consistency in an open-source project, use English.

✏️ Proposed fix
-// FIX: Blindamos el envío del token (Security Risk)
+// Require transport security to protect Bearer token transmission
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// FIX: Blindamos el envío del token (Security Risk)
// Require transport security to protect Bearer token transmission
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cmd/configure.go` at line 95, Replace the Spanish inline comment "// FIX:
Blindamos el envío del token (Security Risk)" with an English equivalent to keep
repo-wide consistency; for example, update the comment near the configure
command in pkg/cmd/configure.go to "// FIX: Protect token transmission (Security
Risk)" or "// FIX: Prevent token exposure during transmission (Security Risk)"
so the intent remains clear and in English.

func (TokenAuth) RequireTransportSecurity() bool {
return true
}

func ConfiguredGRPCClient() ([]grpc.DialOption, string, error) {
credsPath := getCredentialsPath()
data, err := os.ReadFile(credsPath)
if err != nil {
return nil, "", fmt.Errorf("could not read credentials (run 'permify configure' first): %w", err)
}

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

var opts []grpc.DialOption

if creds.APIToken != "" {
opts = append(opts, grpc.WithPerRPCCredentials(TokenAuth{Token: creds.APIToken}))
}

if creds.CertPath != "" && creds.CertKey != "" {
cert, err := tls.LoadX509KeyPair(creds.CertPath, creds.CertKey)
if err != nil {
return nil, "", fmt.Errorf("could not load client key pair: %w", err)
}
// FIX: Forzamos la versión mínima de TLS
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use English for code comments.

This comment is also in Spanish. Use English for consistency.

✏️ Proposed fix
-		// FIX: Forzamos la versión mínima de TLS
+		// Enforce minimum TLS version for security
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// FIX: Forzamos la versión mínima de TLS
// Enforce minimum TLS version for security
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cmd/configure.go` at line 123, Replace the Spanish inline comment
"Forzamos la versión mínima de TLS" with an English equivalent (e.g., "Enforce
minimum TLS version") in the same spot where TLS version is forced so the
codebase comments remain consistent; update any nearby related comments that are
also Spanish to English to keep the section uniform and searchable.

tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
}
opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
} else {
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
Comment on lines +114 to +131
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Configuration conflict: API token with insecure transport will fail at runtime.

When a user configures an API token but no TLS certificates, TokenAuth (with RequireTransportSecurity() = true) is combined with insecure.NewCredentials(). gRPC will reject this combination at dial time with an error like "transport: cannot send secure credentials on an insecure connection".

Consider either:

  1. Returning an explicit error if API token is set without TLS configuration
  2. Supporting server-side TLS (without client certs) by loading system CA roots when only an API token is provided
🛡️ Option 1: Fail early with a clear error message
 	if creds.APIToken != "" {
+		if creds.CertPath == "" || creds.CertKey == "" {
+			return nil, "", fmt.Errorf("API token requires TLS configuration (cert_path and cert_key must be set)")
+		}
 		opts = append(opts, grpc.WithPerRPCCredentials(TokenAuth{Token: creds.APIToken}))
 	}
🛡️ Option 2: Support server-side TLS without client certs
 	if creds.CertPath != "" && creds.CertKey != "" {
 		cert, err := tls.LoadX509KeyPair(creds.CertPath, creds.CertKey)
 		if err != nil {
 			return nil, "", fmt.Errorf("could not load client key pair: %w", err)
 		}
 		// Enforce minimum TLS version for security
 		tlsConfig := &tls.Config{
 			Certificates: []tls.Certificate{cert},
 			MinVersion:   tls.VersionTLS12,
 		}
 		opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
+	} else if creds.APIToken != "" {
+		// Use system CA roots for server-side TLS when API token is set
+		tlsConfig := &tls.Config{
+			MinVersion: tls.VersionTLS12,
+		}
+		opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
 	} else {
 		opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if creds.APIToken != "" {
opts = append(opts, grpc.WithPerRPCCredentials(TokenAuth{Token: creds.APIToken}))
}
if creds.CertPath != "" && creds.CertKey != "" {
cert, err := tls.LoadX509KeyPair(creds.CertPath, creds.CertKey)
if err != nil {
return nil, "", fmt.Errorf("could not load client key pair: %w", err)
}
// FIX: Forzamos la versión mínima de TLS
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
}
opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
} else {
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
if creds.APIToken != "" {
if creds.CertPath == "" || creds.CertKey == "" {
return nil, "", fmt.Errorf("API token requires TLS configuration (cert_path and cert_key must be set)")
}
opts = append(opts, grpc.WithPerRPCCredentials(TokenAuth{Token: creds.APIToken}))
}
if creds.CertPath != "" && creds.CertKey != "" {
cert, err := tls.LoadX509KeyPair(creds.CertPath, creds.CertKey)
if err != nil {
return nil, "", fmt.Errorf("could not load client key pair: %w", err)
}
// FIX: Forzamos la versión mínima de TLS
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
}
opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
} else {
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
Suggested change
if creds.APIToken != "" {
opts = append(opts, grpc.WithPerRPCCredentials(TokenAuth{Token: creds.APIToken}))
}
if creds.CertPath != "" && creds.CertKey != "" {
cert, err := tls.LoadX509KeyPair(creds.CertPath, creds.CertKey)
if err != nil {
return nil, "", fmt.Errorf("could not load client key pair: %w", err)
}
// FIX: Forzamos la versión mínima de TLS
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
}
opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
} else {
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
if creds.APIToken != "" {
opts = append(opts, grpc.WithPerRPCCredentials(TokenAuth{Token: creds.APIToken}))
}
if creds.CertPath != "" && creds.CertKey != "" {
cert, err := tls.LoadX509KeyPair(creds.CertPath, creds.CertKey)
if err != nil {
return nil, "", fmt.Errorf("could not load client key pair: %w", err)
}
// Enforce minimum TLS version for security
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
}
opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
} else if creds.APIToken != "" {
// Use system CA roots for server-side TLS when API token is set
tlsConfig := &tls.Config{
MinVersion: tls.VersionTLS12,
}
opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
} else {
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/cmd/configure.go` around lines 114 - 131, The code allows setting
creds.APIToken while falling back to
grpc.WithTransportCredentials(insecure.NewCredentials()) when creds.CertPath and
creds.CertKey are empty, which will cause TokenAuth
(RequireTransportSecurity()=true) to fail at dial time; fix by detecting this
case and either return an explicit error when creds.APIToken is set but no TLS
is configured (check creds.APIToken && (creds.CertPath=="" ||
creds.CertKey=="")) OR, to support server-side TLS without client certs, create
a tls.Config with MinVersion: tls.VersionTLS12 and RootCAs loaded from system
cert pool (x509.SystemCertPool()) and use credentials.NewTLS(tlsConfig) instead
of insecure.NewCredentials(); update the branch around the TokenAuth and
transport credential construction (refs: TokenAuth, creds.APIToken,
creds.CertPath, creds.CertKey, tls.Config, credentials.NewTLS,
insecure.NewCredentials) accordingly.


endpoint := creds.Endpoint
if endpoint == "" {
endpoint = "localhost:3478"
}

return opts, endpoint, nil
}