Skip to content
Closed
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
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ require (
)

require (
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/manifoldco/promptui v0.9.0 // indirect

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

The manifoldco/promptui dependency should be moved from indirect dependencies to direct dependencies since it's imported directly in the new files api_key.go and mcp_register.go. Indirect dependencies are typically for transitive dependencies only.

The line should be:

require (
    github.com/bmatcuk/doublestar/v4 v4.9.1
    github.com/manifoldco/promptui v0.9.0
    github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8
    github.com/spf13/cobra v1.10.1
)

Copilot uses AI. Check for mistakes.
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/spf13/pflag v1.0.10 // indirect
golang.org/x/sys v0.15.0 // indirect
github.com/stretchr/testify v1.11.1 // indirect
golang.org/x/sys v0.15.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
11 changes: 9 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
github.com/bmatcuk/doublestar/v4 v4.9.1 h1:X8jg9rRZmJd4yRy7ZeNDRnM+T3ZfHv15JiBJ/avrEXE=
github.com/bmatcuk/doublestar/v4 v4.9.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e h1:fY5BOSpyZCqRo5OhCuC+XN+r/bBCmeuuJtjz+bCNIf8=
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/manifoldco/promptui v0.9.0 h1:3V4HzJk1TtXW1MTZMP7mdlwbBpIinw3HztaIlYthEiA=
github.com/manifoldco/promptui v0.9.0/go.mod h1:ka04sppxSGFAtxX0qhlYQjISsg9mR4GWtQEhdbn6Pgg=
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU=
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
Expand All @@ -15,11 +21,12 @@ github.com/spf13/cobra v1.10.1/go.mod h1:7SmJGaTHFVBY0jW4NXGluQoLvhqFQM+6XSKD+P4
github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk=
github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
golang.org/x/sys v0.0.0-20181122145206-62eef0e2fa9b/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc=
golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
269 changes: 269 additions & 0 deletions internal/cmd/api_key.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
package cmd

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

"github.com/manifoldco/promptui"
)

// promptAPIKeySetup prompts user to setup API key (without checking if it exists)
func promptAPIKeySetup() {
promptAPIKeyConfiguration(false)
}

// promptAPIKeyIfNeeded checks if OpenAI API key is configured and prompts if not
func promptAPIKeyIfNeeded() {
promptAPIKeyConfiguration(true)
}

// promptAPIKeyConfiguration handles API key configuration with optional existence check
func promptAPIKeyConfiguration(checkExisting bool) {
envPath := filepath.Join(".sym", ".env")

if checkExisting {
// 1. Check environment variable
if os.Getenv("OPENAI_API_KEY") != "" {
fmt.Println("\nβœ“ OpenAI API key detected from environment")
return
}

// 2. Check .sym/.env file
if hasAPIKeyInEnvFile(envPath) {
fmt.Println("\nβœ“ OpenAI API key found in .sym/.env")
return
}

// Neither found - show warning
fmt.Println("\n⚠ OpenAI API key not found")
fmt.Println(" (Required for convert, validate commands and MCP auto-conversion)")
fmt.Println()
}

// Create selection prompt
items := []string{
"Enter API key",
"Skip (set manually later)",
}

templates := &promptui.SelectTemplates{
Label: "{{ . }}?",
Active: "β–Έ {{ . | cyan }}",
Inactive: " {{ . }}",
Selected: "βœ“ {{ . | green }}",
}

selectPrompt := promptui.Select{
Label: "Would you like to configure it now",
Items: items,
Templates: templates,
Size: 2,
}

index, _, err := selectPrompt.Run()
if err != nil {
fmt.Println("\nSkipped API key configuration")
return
}

switch index {
case 0: // Enter API key
apiKey, err := promptForAPIKey()
if err != nil {
fmt.Printf("\n❌ Failed to read API key: %v\n", err)
return
}

// Validate API key format
if err := validateAPIKey(apiKey); err != nil {
fmt.Printf("\n⚠ Warning: %v\n", err)
fmt.Println(" API key was saved anyway. Make sure it's correct.")
}

// Save to .sym/.env
if err := saveToEnvFile(envPath, "OPENAI_API_KEY", apiKey); err != nil {
fmt.Printf("\n❌ Failed to save API key: %v\n", err)
return
}

fmt.Println("\nβœ“ API key saved to .sym/.env")

// Add to .gitignore
if err := ensureGitignore(".sym/.env"); err != nil {
fmt.Printf("⚠ Warning: Failed to update .gitignore: %v\n", err)
fmt.Println(" Please manually add '.sym/.env' to .gitignore")
} else {
fmt.Println("βœ“ Added .sym/.env to .gitignore")
}

case 1: // Skip
fmt.Println("\nSkipped API key configuration")
fmt.Println("\nπŸ’‘ Tip: You can set OPENAI_API_KEY in:")
fmt.Println(" - .sym/.env file")
fmt.Println(" - System environment variable")
}
}

// promptForAPIKey prompts user to enter API key with masking
func promptForAPIKey() (string, error) {
prompt := promptui.Prompt{
Label: "Enter your OpenAI API key",
Mask: '*',
Validate: func(input string) error {
if len(input) == 0 {
return fmt.Errorf("API key cannot be empty")
}
return nil
},
}

result, err := prompt.Run()
if err != nil {
return "", err
}

return strings.TrimSpace(result), nil
Comment on lines +115 to +128

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

The API key value is trimmed after masking validation, but not before. If a user accidentally includes leading/trailing whitespace when pasting their API key, it will be saved with that whitespace, causing authentication failures. The validation at line 116 checks len(input) == 0 on the untrimmed input.

Consider trimming before validation:

Validate: func(input string) error {
    input = strings.TrimSpace(input)
    if len(input) == 0 {
        return fmt.Errorf("API key cannot be empty")
    }
    return nil
},

Or validate the trimmed result at line 128.

Copilot uses AI. Check for mistakes.
}

// validateAPIKey performs basic validation on API key format
func validateAPIKey(key string) error {
if !strings.HasPrefix(key, "sk-") {
return fmt.Errorf("API key should start with 'sk-'")
}
if len(key) < 20 {

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

The API key validation is too lenient and doesn't account for newer OpenAI API key formats. As of 2023, OpenAI introduced project-based API keys that start with sk-proj- and other formats. The current validation will incorrectly reject valid API keys.

Consider updating the validation to be more flexible:

func validateAPIKey(key string) error {
    if !strings.HasPrefix(key, "sk-") {
        return fmt.Errorf("API key should start with 'sk-'")
    }
    if len(key) < 40 {  // Modern OpenAI keys are typically 48+ chars
        return fmt.Errorf("API key seems too short")
    }
    return nil
}
Suggested change
if len(key) < 20 {
if len(key) < 40 {

Copilot uses AI. Check for mistakes.
return fmt.Errorf("API key seems too short")
}
return nil
}

// hasAPIKeyInEnvFile checks if OPENAI_API_KEY exists in .env file
func hasAPIKeyInEnvFile(envPath string) bool {
file, err := os.Open(envPath)
if err != nil {
return false
}
defer func() { _ = file.Close() }()

scanner := bufio.NewScanner(file)
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())
if strings.HasPrefix(line, "OPENAI_API_KEY=") {
parts := strings.SplitN(line, "=", 2)
if len(parts) == 2 && strings.TrimSpace(parts[1]) != "" {
return true
}
}
}

return false
}

// saveToEnvFile saves a key-value pair to .env file
func saveToEnvFile(envPath, key, value string) error {
// Create .sym directory if it doesn't exist
symDir := filepath.Dir(envPath)
if err := os.MkdirAll(symDir, 0755); err != nil {
return fmt.Errorf("failed to create .sym directory: %w", err)
}

// Read existing content
var lines []string
existingFile, err := os.Open(envPath)
if err == nil {
scanner := bufio.NewScanner(existingFile)
for scanner.Scan() {
line := scanner.Text()
// Skip existing OPENAI_API_KEY lines
if !strings.HasPrefix(strings.TrimSpace(line), key+"=") {
lines = append(lines, line)
}
}
Comment on lines +179 to +183

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The function filters out lines starting with the key prefix but doesn't preserve blank lines or comments in the original .env file. This means any existing structure (like section comments or blank line separators) will be lost when updating the API key.

Consider preserving the original file structure:

// Skip existing OPENAI_API_KEY lines, but preserve comments and blank lines
trimmedLine := strings.TrimSpace(line)
if !strings.HasPrefix(trimmedLine, key+"=") {
    lines = append(lines, line)
}

Copilot uses AI. Check for mistakes.
_ = existingFile.Close()
}

// Add new key
lines = append(lines, fmt.Sprintf("%s=%s", key, value))

// Write to file with restrictive permissions (owner read/write only)
content := strings.Join(lines, "\n") + "\n"
if err := os.WriteFile(envPath, []byte(content), 0600); err != nil {
return fmt.Errorf("failed to write .env file: %w", err)
}

return nil
}

// ensureGitignore ensures that the given path is in .gitignore
func ensureGitignore(path string) error {
gitignorePath := ".gitignore"

// Read existing .gitignore
var lines []string
existingFile, err := os.Open(gitignorePath)
if err == nil {
scanner := bufio.NewScanner(existingFile)
for scanner.Scan() {
line := scanner.Text()
lines = append(lines, line)
// Check if already exists
if strings.TrimSpace(line) == path {
_ = existingFile.Close()
return nil // Already in .gitignore
}
}
_ = existingFile.Close()
}

// Add to .gitignore
lines = append(lines, "", "# Symphony API key configuration", path)
content := strings.Join(lines, "\n") + "\n"

if err := os.WriteFile(gitignorePath, []byte(content), 0644); err != nil {
return fmt.Errorf("failed to update .gitignore: %w", err)
}

Comment on lines +220 to +227

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The ensureGitignore function has a potential race condition. Between checking if the path exists in .gitignore (lines 212-214) and writing to the file (line 224), another process could modify .gitignore, potentially causing the path to be added multiple times or losing other concurrent modifications.

Consider using file locking or an atomic read-modify-write operation if concurrent modifications are a concern in your use case.

Suggested change
// Add to .gitignore
lines = append(lines, "", "# Symphony API key configuration", path)
content := strings.Join(lines, "\n") + "\n"
if err := os.WriteFile(gitignorePath, []byte(content), 0644); err != nil {
return fmt.Errorf("failed to update .gitignore: %w", err)
}
// Before writing, re-read the file to avoid race conditions
// (in case another process added the path in the meantime)
latestLines := []string{}
latestFile, err := os.Open(gitignorePath)
if err == nil {
scanner := bufio.NewScanner(latestFile)
found := false
for scanner.Scan() {
line := scanner.Text()
latestLines = append(latestLines, line)
if strings.TrimSpace(line) == path {
found = true
}
}
_ = latestFile.Close()
if found {
return nil // Already in .gitignore after re-check
}
} else {
// If file doesn't exist, use the original lines (could be empty)
latestLines = lines
}
// Add to .gitignore
latestLines = append(latestLines, "", "# Symphony API key configuration", path)
content := strings.Join(latestLines, "\n") + "\n"
// Write to a temp file and atomically replace .gitignore
tmpFile, err := os.CreateTemp("", "gitignore_tmp")
if err != nil {
return fmt.Errorf("failed to create temp file for .gitignore: %w", err)
}
defer func() { _ = os.Remove(tmpFile.Name()) }()
if _, err := tmpFile.Write([]byte(content)); err != nil {
_ = tmpFile.Close()
return fmt.Errorf("failed to write to temp .gitignore: %w", err)
}
if err := tmpFile.Close(); err != nil {
return fmt.Errorf("failed to close temp .gitignore: %w", err)
}
if err := os.Rename(tmpFile.Name(), gitignorePath); err != nil {
return fmt.Errorf("failed to atomically update .gitignore: %w", err)
}

Copilot uses AI. Check for mistakes.
return nil
}

// getAPIKey retrieves OpenAI API key from environment or .env file
// Priority: 1) System environment variable 2) .sym/.env file
func getAPIKey() (string, error) {
// 1. Check system environment variable first
if key := os.Getenv("OPENAI_API_KEY"); key != "" {
return key, nil
}

// 2. Check .sym/.env file
envPath := filepath.Join(".sym", ".env")
key, err := loadFromEnvFile(envPath, "OPENAI_API_KEY")
if err == nil && key != "" {
return key, nil
}

return "", fmt.Errorf("OPENAI_API_KEY not found in environment or .sym/.env")
}

// loadFromEnvFile loads a specific key from .env file
func loadFromEnvFile(envPath, key string) (string, error) {
file, err := os.Open(envPath)
if err != nil {
return "", err
}
defer func() { _ = file.Close() }()

scanner := bufio.NewScanner(file)
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())
if strings.HasPrefix(line, key+"=") {
parts := strings.SplitN(line, "=", 2)
if len(parts) == 2 {
return strings.TrimSpace(parts[1]), nil
}
}
}

return "", fmt.Errorf("key %s not found in %s", key, envPath)
}
7 changes: 4 additions & 3 deletions internal/cmd/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,10 @@ func runMultiTargetConvert(userPolicy *schema.UserPolicy) error {
}

// Setup OpenAI client
apiKey := os.Getenv("OPENAI_API_KEY")
if apiKey == "" {
fmt.Println("Warning: OPENAI_API_KEY not set, using fallback inference")
apiKey, err := getAPIKey()
if err != nil {
fmt.Printf("Warning: %v, using fallback inference\n", err)
apiKey = ""
}

timeout := time.Duration(convertTimeout) * time.Second
Expand Down
36 changes: 35 additions & 1 deletion internal/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,37 @@ This command:
Run: runInit,
}

var initForce bool
var (
initForce bool
skipMCPRegister bool
registerMCPOnly bool
skipAPIKey bool
setupAPIKeyOnly bool
)

func init() {
initCmd.Flags().BoolVarP(&initForce, "force", "f", false, "Overwrite existing roles.json")
initCmd.Flags().BoolVar(&skipMCPRegister, "skip-mcp", false, "Skip MCP server registration prompt")
initCmd.Flags().BoolVar(&registerMCPOnly, "register-mcp", false, "Register MCP server only (skip roles/policy init)")
initCmd.Flags().BoolVar(&skipAPIKey, "skip-api-key", false, "Skip OpenAI API key configuration prompt")
initCmd.Flags().BoolVar(&setupAPIKeyOnly, "setup-api-key", false, "Setup OpenAI API key only (skip roles/policy init)")
}

func runInit(cmd *cobra.Command, args []string) {
// MCP registration only mode
if registerMCPOnly {
fmt.Println("πŸ”§ Registering Symphony MCP server...")
promptMCPRegistration()
return
}

// API key setup only mode
if setupAPIKeyOnly {
fmt.Println("πŸ”‘ Setting up OpenAI API key...")
promptAPIKeySetup()
return
}
Comment on lines 47 to +60

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

Missing validation for mutually exclusive flags. A user could specify both --register-mcp and --setup-api-key simultaneously, or combine either with --skip-mcp or --skip-api-key, which would create confusing behavior since only the first check executes and returns.

Consider adding validation:

func runInit(cmd *cobra.Command, args []string) {
    // Validate flag combinations
    if registerMCPOnly && setupAPIKeyOnly {
        fmt.Println("❌ Error: Cannot use --register-mcp and --setup-api-key together")
        os.Exit(1)
    }
    if registerMCPOnly && skipMCPRegister {
        fmt.Println("❌ Error: Cannot use --register-mcp and --skip-mcp together")
        os.Exit(1)
    }
    if setupAPIKeyOnly && skipAPIKey {
        fmt.Println("❌ Error: Cannot use --setup-api-key and --skip-api-key together")
        os.Exit(1)
    }
    
    // ... rest of function
}

Copilot uses AI. Check for mistakes.

// Check if logged in
if !config.IsLoggedIn() {
fmt.Println("❌ Not logged in")
Expand Down Expand Up @@ -115,6 +139,16 @@ func runInit(cmd *cobra.Command, args []string) {
fmt.Println(" 2. Commit: git add .sym/ && git commit -m 'Initialize Symphony roles and policy'")
fmt.Println(" 3. Push: git push")
fmt.Println("\nAfter pushing, team members can clone and use 'sym my-role' to check their access.")

// MCP registration prompt
if !skipMCPRegister {
promptMCPRegistration()
}

// API key configuration prompt
if !skipAPIKey {
promptAPIKeyIfNeeded()
}
}

// createDefaultPolicy creates a default policy file with RBAC roles
Expand Down
6 changes: 3 additions & 3 deletions internal/cmd/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ func autoConvertPolicy(userPolicyPath, codePolicyPath string) error {
}

// Setup LLM client
apiKey := os.Getenv("OPENAI_API_KEY")
if apiKey == "" {
return fmt.Errorf("OPENAI_API_KEY environment variable not set")
apiKey, err := getAPIKey()
if err != nil {
return fmt.Errorf("OpenAI API key not configured: %w\nTip: Run 'sym init' or set OPENAI_API_KEY in .sym/.env", err)

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

The error message includes a newline character (\n) in the middle, which may not render correctly when wrapped with fmt.Errorf. Consider splitting this into separate fmt.Printf and return statements, or use a multiline string without \n in the format string.

Suggested fix:

if err != nil {
    return fmt.Errorf("OpenAI API key not configured: %w. Tip: Run 'sym init' or set OPENAI_API_KEY in .sym/.env", err)
}
Suggested change
return fmt.Errorf("OpenAI API key not configured: %w\nTip: Run 'sym init' or set OPENAI_API_KEY in .sym/.env", err)
return fmt.Errorf("OpenAI API key not configured: %w. Tip: Run 'sym init' or set OPENAI_API_KEY in .sym/.env", err)

Copilot uses AI. Check for mistakes.
}

llmClient := llm.NewClient(apiKey,
Expand Down
Loading
Loading