Skip to content
Merged
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
8 changes: 4 additions & 4 deletions internal/launcher/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ func GetOrLaunch(l *Launcher, serverID string) (*mcp.Connection, error) {
}

// Log the command being executed
logger.LogInfo("backend", "Launching MCP backend server: %s, command=%s, args=%v", serverID, serverCfg.Command, serverCfg.Args)
logger.LogInfo("backend", "Launching MCP backend server: %s, command=%s, args=%v", serverID, serverCfg.Command, sanitize.SanitizeArgs(serverCfg.Args))
log.Printf("[LAUNCHER] Starting MCP server: %s", serverID)
log.Printf("[LAUNCHER] Command: %s", serverCfg.Command)
log.Printf("[LAUNCHER] Args: %v", serverCfg.Args)
log.Printf("[LAUNCHER] Args: %v", sanitize.SanitizeArgs(serverCfg.Args))
logLauncher.Printf("Launching new server: serverID=%s, command=%s, inContainer=%v, isDirectCommand=%v",
serverID, serverCfg.Command, l.runningInContainer, isDirectCommand)

Expand Down Expand Up @@ -237,10 +237,10 @@ func GetOrLaunchForSession(l *Launcher, serverID, sessionID string) (*mcp.Connec
}

// Log the command being executed
logger.LogInfo("backend", "Launching MCP backend server for session: server=%s, session=%s, command=%s, args=%v", serverID, sessionID, serverCfg.Command, serverCfg.Args)
logger.LogInfo("backend", "Launching MCP backend server for session: server=%s, session=%s, command=%s, args=%v", serverID, sessionID, serverCfg.Command, sanitize.SanitizeArgs(serverCfg.Args))
log.Printf("[LAUNCHER] Starting MCP server for session: %s (session: %s)", serverID, sessionID)
log.Printf("[LAUNCHER] Command: %s", serverCfg.Command)
log.Printf("[LAUNCHER] Args: %v", serverCfg.Args)
log.Printf("[LAUNCHER] Args: %v", sanitize.SanitizeArgs(serverCfg.Args))
logLauncher.Printf("Launching new session server: serverID=%s, sessionID=%s, command=%s", serverID, sessionID, serverCfg.Command)

// Check for environment variable passthrough
Expand Down
32 changes: 32 additions & 0 deletions internal/logger/sanitize/sanitize.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,35 @@ func SanitizeJSON(payloadBytes []byte) json.RawMessage {
compactBytes, _ := json.Marshal(tmp)
return json.RawMessage(compactBytes)
}

// SanitizeArgs returns a sanitized version of command arguments for safe logging.
// It specifically handles Docker-style environment variable arguments (-e VAR=VALUE)
// by truncating the values to prevent exposing sensitive data like API tokens.
// Other arguments are passed through unchanged.
func SanitizeArgs(args []string) []string {
if len(args) == 0 {
return args
}

sanitized := make([]string, len(args))
for i := 0; i < len(args); i++ {
arg := args[i]

// Check if this is an environment variable value after a -e flag
// Format: -e VAR=VALUE
if i > 0 && args[i-1] == "-e" && strings.Contains(arg, "=") {
// Split on first = to get VAR and VALUE
parts := strings.SplitN(arg, "=", 2)
if len(parts) == 2 {
// Truncate the value part
sanitized[i] = parts[0] + "=" + TruncateSecret(parts[1])
} else {
sanitized[i] = arg
}
} else {
// Pass through unchanged
sanitized[i] = arg
}
}
return sanitized
}
153 changes: 153 additions & 0 deletions internal/logger/sanitize/sanitize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,3 +495,156 @@ func TestTruncateSecretMap(t *testing.T) {
})
}
}

func TestSanitizeArgs(t *testing.T) {
tests := []struct {
name string
input []string
expected []string
}{
{
name: "nil args",
input: nil,
expected: nil,
},
{
name: "empty args",
input: []string{},
expected: []string{},
},
{
name: "args without env vars",
input: []string{"run", "--rm", "-i", "image:latest"},
expected: []string{"run", "--rm", "-i", "image:latest"},
},
{
name: "single env var with long token",
input: []string{"run", "-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_1234567890123456789012345678901234567890"},
expected: []string{"run", "-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_..."},
},
{
name: "multiple env vars with different lengths",
input: []string{
"run",
"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_1234567890123456789012345678901234567890",
"-e", "API_KEY=sk_test_1234567890",
"-e", "SHORT=abc",
"--rm",
"-i",
"image:latest",
},
expected: []string{
"run",
"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...",
"-e", "API_KEY=sk_t...",
"-e", "SHORT=...",
"--rm",
"-i",
"image:latest",
},
},
{
name: "realistic docker command with secrets",
input: []string{
"run",
"--rm",
"-i",
"-e", "NO_COLOR=1",
"-e", "TERM=dumb",
"-e", "PYTHONUNBUFFERED=1",
"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_abcdefghijklmnopqrstuvwxyz1234567890",
"-e", "GITHUB_READ_ONLY=1",
"ghcr.io/github/github-mcp-server:v0.29.0",
},
expected: []string{
"run",
"--rm",
"-i",
"-e", "NO_COLOR=...",
"-e", "TERM=...",
"-e", "PYTHONUNBUFFERED=...",
"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...",
"-e", "GITHUB_READ_ONLY=...",
"ghcr.io/github/github-mcp-server:v0.29.0",
},
},
{
name: "env var without equals sign",
input: []string{"run", "-e", "MYVAR", "image:latest"},
expected: []string{"run", "-e", "MYVAR", "image:latest"},
},
{
name: "env var with empty value",
input: []string{"run", "-e", "EMPTY=", "image:latest"},
expected: []string{"run", "-e", "EMPTY=", "image:latest"},
},
{
name: "env var with equals in value",
input: []string{
"run",
"-e", "CONFIG=key=value=extra",
},
expected: []string{
"run",
"-e", "CONFIG=key=...",
},
},
{
name: "-e flag at end without value",
input: []string{"run", "--rm", "-e"},
expected: []string{"run", "--rm", "-e"},
},
{
name: "mixed flags and env vars",
input: []string{
"run",
"--name", "test-container",
"-e", "API_KEY=secret123456",
"--network", "host",
"-e", "TOKEN=mytoken",
"image:latest",
},
expected: []string{
"run",
"--name", "test-container",
"-e", "API_KEY=secr...",
"--network", "host",
"-e", "TOKEN=myto...",
"image:latest",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := SanitizeArgs(tt.input)
assert.Equal(t, tt.expected, result)
})
}
}

func TestSanitizeArgsDoesNotLeakSecrets(t *testing.T) {
// Test that actual secrets are not present in sanitized output
secretToken := "ghp_verysecrettokenthatshouldbehidden1234567890"
secretApiKey := "test_secret_key_1234567890abcdefghijklmnop"

input := []string{
"run",
"-e", fmt.Sprintf("GITHUB_TOKEN=%s", secretToken),
"-e", fmt.Sprintf("API_KEY=%s", secretApiKey),
"image:latest",
}

result := SanitizeArgs(input)

// Convert result to string for easy checking
resultStr := fmt.Sprintf("%v", result)

// Verify secrets are NOT in the output
assert.NotContains(t, resultStr, secretToken, "Secret token should not be present in sanitized output")
assert.NotContains(t, resultStr, secretApiKey, "Secret API key should not be present in sanitized output")

// Verify truncated versions ARE present
assert.Contains(t, resultStr, "GITHUB_TOKEN=ghp_...", "Truncated token should be present")
assert.Contains(t, resultStr, "API_KEY=test...", "Truncated API key should be present")
}
13 changes: 7 additions & 6 deletions internal/mcp/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"time"

"github.com/githubnext/gh-aw-mcpg/internal/logger"
"github.com/githubnext/gh-aw-mcpg/internal/logger/sanitize"
sdk "github.com/modelcontextprotocol/go-sdk/mcp"
)

Expand Down Expand Up @@ -153,8 +154,8 @@ func setupHTTPRequest(ctx context.Context, url string, requestBody []byte, heade

// NewConnection creates a new MCP connection using the official SDK
func NewConnection(ctx context.Context, command string, args []string, env map[string]string) (*Connection, error) {
logger.LogInfo("backend", "Creating new MCP backend connection, command=%s, args=%v", command, args)
logConn.Printf("Creating new MCP connection: command=%s, args=%v", command, args)
logger.LogInfo("backend", "Creating new MCP backend connection, command=%s, args=%v", command, sanitize.SanitizeArgs(args))
logConn.Printf("Creating new MCP connection: command=%s, args=%v", command, sanitize.SanitizeArgs(args))
ctx, cancel := context.WithCancel(ctx)

// Create MCP client
Expand All @@ -178,8 +179,8 @@ func NewConnection(ctx context.Context, command string, args []string, env map[s
}
}

logger.LogInfo("backend", "Starting MCP backend server, command=%s, args=%v", command, expandedArgs)
log.Printf("Starting MCP server command: %s %v", command, expandedArgs)
logger.LogInfo("backend", "Starting MCP backend server, command=%s, args=%v", command, sanitize.SanitizeArgs(expandedArgs))
log.Printf("Starting MCP server command: %s %v", command, sanitize.SanitizeArgs(expandedArgs))
transport := &sdk.CommandTransport{Command: cmd}

// Connect to the server (this handles the initialization handshake automatically)
Expand All @@ -190,10 +191,10 @@ func NewConnection(ctx context.Context, command string, args []string, env map[s
cancel()

// Enhanced error context for debugging
logger.LogErrorMd("backend", "MCP backend connection failed, command=%s, args=%v, error=%v", command, expandedArgs, err)
logger.LogErrorMd("backend", "MCP backend connection failed, command=%s, args=%v, error=%v", command, sanitize.SanitizeArgs(expandedArgs), err)
log.Printf("❌ MCP Connection Failed:")
log.Printf(" Command: %s", command)
log.Printf(" Args: %v", expandedArgs)
log.Printf(" Args: %v", sanitize.SanitizeArgs(expandedArgs))
log.Printf(" Error: %v", err)

// Check if it's a command not found error
Expand Down