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
9 changes: 4 additions & 5 deletions mcp/client_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestRenderClientListEscapesMarkdownCells(t *testing.T) {
}

func TestAddOAuthNoVerifyPersistsConfiguration(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", t.TempDir())
isolateConfigHome(t)
output, err := executeMCPCommand(
"add", "private", "https://example.com/mcp",
"--oauth-client-id", "client-1",
Expand All @@ -53,16 +53,15 @@ func TestAddOAuthNoVerifyPersistsConfiguration(t *testing.T) {
}

func TestAddNoBrowserRequiresOAuth(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", t.TempDir())
isolateConfigHome(t)
_, err := executeMCPCommand("add", "public", "https://example.com/mcp", "--no-browser", "--no-verify")
if err == nil || !strings.Contains(err.Error(), "requires --oauth") {
t.Fatalf("error = %v", err)
}
}

func TestAddRejectsInvalidNameBeforeOAuthStorageBinding(t *testing.T) {
configHome := t.TempDir()
t.Setenv("XDG_CONFIG_HOME", configHome)
isolateConfigHome(t)
_, err := executeMCPCommand("add", "../config", "https://example.com/mcp", "--oauth", "--no-verify")
if err == nil || !strings.Contains(err.Error(), "invalid server name") {
t.Fatalf("error = %v", err)
Expand All @@ -74,7 +73,7 @@ func TestAddRejectsInvalidNameBeforeOAuthStorageBinding(t *testing.T) {
}

func TestAddRejectsLiteralOAuthSecret(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", t.TempDir())
isolateConfigHome(t)
_, err := executeMCPCommand(
"add", "private", "https://example.com/mcp", "--oauth-client-id", "client", "--oauth-client-secret", "literal", "--no-verify",
)
Expand Down
2 changes: 1 addition & 1 deletion mcp/client_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
)

func TestClientCommandsStdioAndOfflineHelp(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", t.TempDir())
isolateConfigHome(t)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 8 \
  'go run|executeMCPCommand|exec\.Cmd|GOPATH|GOMODCACHE|Env:' \
  mcp --glob '*.go' --glob 'go.mod'

Repository: flanksource/clicky

Length of output: 14017


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- isolateConfigHome definition and uses ---'
rg -n -C 12 'func isolateConfigHome|isolateConfigHome\(' . --glob '*.go'

printf '%s\n' '--- subprocess construction and environment handling ---'
rg -n -C 14 'exec\.Command(Context)?|Cmd\.Env|os\.Environ|GOPATH|GOMODCACHE|HOME|ServerConfig|Command:' mcp --glob '*.go'

printf '%s\n' '--- module and workspace metadata ---'
find . -maxdepth 2 \( -name go.mod -o -name go.work \) -print -exec sed -n '1,100p' {} \;

Repository: flanksource/clicky

Length of output: 50375


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- stdio transport implementation ---'
sed -n '83,135p' mcp/client_dial.go

printf '%s\n' '--- test server module metadata ---'
find mcp/testdata -maxdepth 3 -type f -print -exec sh -c 'echo "--- $1"; sed -n "1,120p" "$1"' sh {} \;

printf '%s\n' '--- dependency versions and Go directives ---'
sed -n '1,140p' go.mod
rg -n 'New.*Stdio|Stdio|Command.*Args|cfg\.Env|exec\.' . --glob '*.go' --glob '!vendor/**'

Repository: flanksource/clicky

Length of output: 19514


🌐 Web query:

github mark3labs mcp-go v0.56.0 transport.NewStdio environment os.Environ

💡 Result:

In the mark3labs/mcp-go library, specifically as of version 0.56.0, the transport.NewStdio function (and related helpers) handles environment variables by merging provided custom environment variables with the host process's existing environment variables [1][2]. When spawning a subprocess, the transport implementation uses the following logic to set the environment [1]: cmd.Env = append(os.Environ, c.env...) This means: 1. It retrieves the host's current environment variables using os.Environ [1]. 2. It appends the slice of custom environment variables (provided via the env argument in NewStdio or NewStdioWithOptions) to the host environment [1]. 3. The resulting environment is then passed to the exec.Cmd instance used to launch the subprocess [1]. If you need finer control over the process creation—such as replacing the environment entirely rather than appending to it, or configuring other low-level system attributes—the library provides the NewStdioWithOptions function [1][3]. By using this with the WithCommandFunc option, you can provide a custom factory function to define exactly how the exec.Cmd is constructed and configured, including full control over its Env field [4][3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

url='https://raw.githubusercontent.com/mark3labs/mcp-go/v0.56.0/client/transport/stdio.go'
if command -v curl >/dev/null 2>&1; then
  curl -fsSL "$url" | sed -n '1,240p'
else
  echo 'curl unavailable' >&2
  exit 1
fi

Repository: flanksource/clicky

Length of output: 6996


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import os
import subprocess
import tempfile

def go_env(home, extra):
    env = os.environ.copy()
    env["HOME"] = home
    env.pop("GOPATH", None)
    env.pop("GOMODCACHE", None)
    env.update(extra)
    out = subprocess.check_output(
        ["go", "env", "GOPATH", "GOMODCACHE"],
        env=env,
        text=True,
    ).splitlines()
    return dict(zip(("GOPATH", "GOMODCACHE"), out))

with tempfile.TemporaryDirectory() as home:
    print("unset GOPATH/GOMODCACHE:", go_env(home, {}))
    print("fixed GOPATH:", go_env(home, {"GOPATH": "/stable/gopath"}))
    print("fixed GOMODCACHE:", go_env(home, {"GOMODCACHE": "/stable/modcache"}))

print("parent GOPATH:", os.environ.get("GOPATH", "<unset>"))
print("parent GOMODCACHE:", os.environ.get("GOMODCACHE", "<unset>"))
PY

Repository: flanksource/clicky

Length of output: 507


Preserve GOPATH or GOMODCACHE

When both variables are unset, changing HOME makes the child go run use a temporary module cache. Preserve one variable or isolate application configuration without changing HOME.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mcp/client_e2e_test.go` at line 16, Update isolateConfigHome to avoid
changing HOME when both GOPATH and GOMODCACHE are unset, preserving the child go
run’s existing module cache behavior. Instead, preserve one of those environment
variables or isolate only the application configuration while leaving HOME
unchanged.

workingDir, err := os.Getwd()
if err != nil {
t.Fatal(err)
Expand Down
2 changes: 1 addition & 1 deletion mcp/client_oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func testOAuthLoginUsesOIDCDiscoveryAndRefreshes(t *testing.T, metadataOverride
defer httpServer.Close()
baseURL = httpServer.URL

t.Setenv("XDG_CONFIG_HOME", t.TempDir())
isolateConfigHome(t)
registry := NewServerRegistry("testapp")
oauthConfig := &OAuthClientConfig{Scopes: append([]string(nil), scopes...)}
if metadataOverride {
Expand Down
12 changes: 12 additions & 0 deletions mcp/client_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ import (
"time"
)

// isolateConfigHome points NewServerRegistry at a per-test directory on every
// platform. os.UserConfigDir only reads XDG_CONFIG_HOME on unix-like systems;
// on darwin it derives ~/Library/Application Support from HOME, so setting only
// XDG_CONFIG_HOME leaks registry, cache, and OAuth state into the real user
// config directory and makes later runs fail on stale state.
func isolateConfigHome(t *testing.T) {
t.Helper()
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config"))
}
Comment on lines +13 to +23

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 5 \
  'User(Config|Cache|Home)Dir|XDG_(CONFIG|CACHE)_HOME|AppData|LocalAppData|USERPROFILE|GOOS|windows' \
  --glob '*.go' --glob 'go.mod' --glob '*.yml' --glob '*.yaml' --glob 'Dockerfile*' .

Repository: flanksource/clicky

Length of output: 16977


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- test helper and registry call sites ---'
cat -n mcp/client_registry_test.go | sed -n '1,220p'
printf '%s\n' '--- registry path implementation ---'
cat -n mcp/client_registry.go | sed -n '235,275p'
printf '%s\n' '--- related config/cache/home helpers ---'
rg -n -C 8 'GetConfigPathFor|GetPromptsPathFor|UserConfigDir|UserCacheDir|UserHomeDir|\\.cache|mcp-config|mcp-prompts|OAuth|oauth' mcp ai --glob '*.go'
printf '%s\n' '--- Go stdlib source locations and relevant implementations ---'
if command -v go >/dev/null 2>&1; then
  goroot="$(go env GOROOT)"
  printf 'GOROOT=%s\n' "$goroot"
  rg -n -C 12 'func User(Config|Cache|Home)Dir|XDG_CONFIG_HOME|XDG_CACHE_HOME|LocalAppData|APPDATA|USERPROFILE|Library/Application Support' \
    "$goroot/src/os" "$goroot/src/internal" 2>/dev/null || true
else
  printf '%s\n' 'go command unavailable'
fi

Repository: flanksource/clicky

Length of output: 50375


🏁 Script executed:

#!/bin/bash
set -euo pipefail

if ! command -v go >/dev/null 2>&1; then
  echo "go command unavailable"
  exit 0
fi

goroot="$(go env GOROOT)"
printf '%s\n' '--- UserConfigDir/UserCacheDir/UserHomeDir implementations ---'
rg -l 'func User(Config|Cache|Home)Dir' "$goroot/src/os" | while IFS= read -r f; do
  echo "FILE: $f"
  rg -n -A45 -B5 'func User(Config|Cache|Home)Dir' "$f"
done

printf '%s\n' '--- helper call sites ---'
rg -n -C 3 'isolateConfigHome\(' mcp --glob '*.go'

printf '%s\n' '--- all NewServerRegistry uses in tests ---'
rg -n -C 3 'NewServerRegistry\(' --glob '*_test.go' .

Repository: flanksource/clicky

Length of output: 11228


Redirect Windows user directories in isolateConfigHome.

On Windows, os.UserConfigDir() reads %AppData% and os.UserHomeDir() reads %USERPROFILE%. Set AppData and USERPROFILE to paths under home; otherwise tests can write registry, cache, or OAuth state to the real user directory.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mcp/client_registry_test.go` around lines 13 - 23, Update isolateConfigHome
to set the Windows environment variables AppData and USERPROFILE to directories
under the test’s temporary home, alongside HOME and XDG_CONFIG_HOME. Ensure
os.UserConfigDir and os.UserHomeDir resolve entirely within the isolated test
directory on Windows.


func TestServerConfigValidate(t *testing.T) {
tests := []struct {
name string
Expand Down
2 changes: 1 addition & 1 deletion mcp/client_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

func TestRunShortHelpListsServers(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", t.TempDir())
isolateConfigHome(t)
registry := NewServerRegistry("testapp")
if err := registry.Add("demo", ServerConfig{Type: "stdio", Command: "server"}); err != nil {
t.Fatal(err)
Expand Down
Loading