diff --git a/docs/git-credential-adapter.md b/docs/git-credential-adapter.md index c45e64cf..96438e22 100644 --- a/docs/git-credential-adapter.md +++ b/docs/git-credential-adapter.md @@ -110,10 +110,10 @@ Stored `gitea.example.com:8443` matches: If you run a self-hosted server on the scheme's default port (443 HTTPS, 22 SSH), omit the port. If you run on a non-default port, include it. -When no stored credential matches the resolved remote host, the adapter falls -through to the un-credentialed path: `git` runs with whatever credentials the -calling shell already has (typically none, in which case the remote will reject -the operation with a 401/403). +When no typed PAT/SSH credential is selected, or the selected credential cannot +match the resolved remote host, adapter-managed remote commands fail closed +with a GoClaw diagnostic. `git` is not allowed to fall through to an +interactive username/password prompt in agent runtime. ## Security model diff --git a/docs/journals/260611-secure-cli-github-credential-runtime-fix.md b/docs/journals/260611-secure-cli-github-credential-runtime-fix.md new file mode 100644 index 00000000..5091fdb2 --- /dev/null +++ b/docs/journals/260611-secure-cli-github-credential-runtime-fix.md @@ -0,0 +1,31 @@ +# Secure CLI GitHub Credential Runtime Fix + +## Summary + +Fixed GitHub-related SecureCLI auth diagnostics for `git` and `gh` runtime +commands linked to digitopvn/goclaw#138 and digitopvn/goclaw#151. + +## Root Cause + +- Adapter-managed `git` remote commands could reach raw Git when no typed PAT + or SSH credential was selected. +- Required env validation only covered `rapidapi`, so credentialed `gh` + commands without `GH_TOKEN` could reach raw GitHub CLI auth guidance. + +## Changes + +- Added fail-closed git credential readiness validation before adapter prepare. +- Generalized required env checks to preset non-optional env vars. +- Added regression tests for missing git typed credentials and missing + `GH_TOKEN`. +- Updated git adapter docs and project changelog. + +## Validation + +- `/usr/local/go/bin/go test ./internal/tools` +- `/usr/local/go/bin/go build ./...` +- `/usr/local/go/bin/go build -tags sqliteonly ./...` + +## Unresolved Questions + +None. diff --git a/docs/project-changelog.md b/docs/project-changelog.md index 160b45da..2959092f 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -4,6 +4,25 @@ Significant changes, features, and fixes in reverse chronological order. --- +## 2026-06-11 + +### Secure CLI GitHub credential runtime diagnostics (issues #138, #151) + +**Fixes** + +- Git remote commands now fail closed before raw `git` auth prompts when no + typed host-scoped PAT/SSH credential is selected. +- Required SecureCLI env validation now applies to preset env vars such as + `GH_TOKEN`, so credentialed `gh` commands report missing credential config + instead of raw `gh auth login` guidance. + +**Tests** + +- Added regressions for `git push` without typed credentials and `gh` without + `GH_TOKEN`. + +--- + ## 2026-06-09 ### Behavior UX sidecar delivery overrides (issue #144) diff --git a/internal/tools/credential_presets.go b/internal/tools/credential_presets.go index ef70cf76..62a31555 100644 --- a/internal/tools/credential_presets.go +++ b/internal/tools/credential_presets.go @@ -173,9 +173,6 @@ func ListPresetNames() []string { func requiredCredentialEnvVars(binary string) []string { name := normalizeBinaryName(binary) - if name != "rapidapi" { - return nil - } preset := GetPreset(name) if preset == nil { return nil diff --git a/internal/tools/credential_presets_rapidapi_test.go b/internal/tools/credential_presets_rapidapi_test.go index 381aaa5a..800f2dd8 100644 --- a/internal/tools/credential_presets_rapidapi_test.go +++ b/internal/tools/credential_presets_rapidapi_test.go @@ -45,11 +45,11 @@ func TestRapidAPIPresetBlocksVerboseSecretLeakFlags(t *testing.T) { } } -func TestRequiredCredentialEnvVarsScopedToRapidAPI(t *testing.T) { +func TestRequiredCredentialEnvVarsFromPresets(t *testing.T) { if got := requiredCredentialEnvVars("rapidapi"); !slices.Contains(got, "RAPIDAPI_KEY") { t.Fatalf("requiredCredentialEnvVars(rapidapi) = %v, missing RAPIDAPI_KEY", got) } - if got := requiredCredentialEnvVars("gh"); len(got) != 0 { - t.Fatalf("requiredCredentialEnvVars(gh) = %v, want no behavior change for existing presets", got) + if got := requiredCredentialEnvVars("gh"); !slices.Contains(got, "GH_TOKEN") { + t.Fatalf("requiredCredentialEnvVars(gh) = %v, missing GH_TOKEN", got) } } diff --git a/internal/tools/credentialed_exec.go b/internal/tools/credentialed_exec.go index 916e0a36..bc4f731e 100644 --- a/internal/tools/credentialed_exec.go +++ b/internal/tools/credentialed_exec.go @@ -465,6 +465,9 @@ func (t *ExecTool) executeCredentialed(ctx context.Context, cred *store.SecureCL if adapter.ShouldInject(args) { userCred := userCredFromBinary(ctx, cred) + if r := validateAdapterCredentialReady(adapter.Name(), binary, userCred); r != nil { + return r + } // Plant the resolved exec cwd so adapters (e.g. git) can run any // pre-flight sub-exec from the right repo, not goclaw's daemon CWD. prepareCtx := WithExecCwd(ctx, cwd) @@ -542,6 +545,26 @@ func credentialSubjectForAdapter(ctx context.Context, bin *store.SecureCLIBinary return store.CredentialUserIDFromContext(ctx) } +func validateAdapterCredentialReady(adapterName, binary string, cred *store.SecureCLIUserCredential) *Result { + if adapterName != "git" { + return nil + } + if cred == nil || cred.CredentialType == nil || strings.TrimSpace(*cred.CredentialType) == "" { + return credentialedGitCredentialResolutionError(binary, "no typed git credential selected") + } + switch strings.TrimSpace(*cred.CredentialType) { + case "pat", "ssh_key": + if cred.HostScope == nil || strings.TrimSpace(*cred.HostScope) == "" { + return credentialedGitCredentialResolutionError(binary, "typed git credential is missing host_scope") + } + return nil + case "env": + return credentialedGitCredentialResolutionError(binary, "legacy env credential cannot authenticate adapter-managed git remotes") + default: + return nil + } +} + func effectiveHostScope(bin *store.SecureCLIBinary) *string { if bin == nil { return nil @@ -908,6 +931,17 @@ func credentialedMissingEnvError(binary string, keys []string) *Result { } } +func credentialedGitCredentialResolutionError(binary, reason string) *Result { + return &Result{ + ForLLM: fmt.Sprintf("[CREDENTIALED EXEC] Git credential resolution failed.\n"+ + "Binary: %s\nReason: %s\n"+ + "Configure a SecureCLI git credential with Credential Type = Personal Access Token or SSH Private Key and Host Scope matching the remote host, then grant this binary to the target agent.", + binary, reason), + ForUser: "Git credential resolution failed. Configure a host-scoped PAT or SSH key for this agent.", + IsError: true, + } +} + func credentialedExecFailError(binary string, args []string, exitCode int, output string) *Result { return &Result{ ForLLM: fmt.Sprintf("[CREDENTIALED EXEC] Command failed (exit code %d).\n"+ diff --git a/internal/tools/credentialed_exec_env_test.go b/internal/tools/credentialed_exec_env_test.go index daaca6d7..609ad15d 100644 --- a/internal/tools/credentialed_exec_env_test.go +++ b/internal/tools/credentialed_exec_env_test.go @@ -183,3 +183,72 @@ func TestExec_RapidAPIWithRequiredEnvReachesDirectExec(t *testing.T) { t.Fatalf("credential value leaked into output: %s", result.ForLLM) } } + +func TestExec_GHMissingRequiredEnvFailsBeforeRawAuth(t *testing.T) { + stub := newStubSecureCLIStore() + stub.byName["gh"] = &store.SecureCLIBinary{ + BinaryName: "gh", + EncryptedEnv: []byte("{}"), + TimeoutSeconds: 10, + DenyArgs: json.RawMessage("[]"), + DenyVerbose: json.RawMessage("[]"), + } + + tool := NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(stub) + + ctx := store.WithTenantID(store.WithAgentID(context.Background(), uuid.New()), uuid.New()) + result := tool.Execute(ctx, map[string]any{"command": "gh issue list --repo digitopvn/goclaw --limit 1"}) + + if !result.IsError { + t.Fatalf("expected missing GH_TOKEN to fail") + } + if !strings.Contains(result.ForLLM, "missing required credential env") { + t.Fatalf("expected actionable missing-env error, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "GH_TOKEN") { + t.Fatalf("expected GH_TOKEN in error, got: %s", result.ForLLM) + } + if strings.Contains(result.ForLLM, "gh auth login") { + t.Fatalf("raw gh auth guidance leaked through: %s", result.ForLLM) + } +} + +func TestExec_GitRemoteCommandWithoutTypedCredentialFailsBeforeRawAuth(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell script fixture is POSIX-only") + } + + binDir := t.TempDir() + binPath := filepath.Join(binDir, "git") + if err := os.WriteFile(binPath, []byte("#!/bin/sh\nprintf 'raw git reached\\n' >&2\nexit 128\n"), 0o755); err != nil { + t.Fatal(err) + } + adapterName := "git" + stub := newStubSecureCLIStore() + stub.byName["git"] = &store.SecureCLIBinary{ + BinaryName: "git", + BinaryPath: &binPath, + EncryptedEnv: []byte("{}"), + TimeoutSeconds: 10, + DenyArgs: json.RawMessage("[]"), + DenyVerbose: json.RawMessage("[]"), + AdapterName: &adapterName, + } + + tool := NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(stub) + + ctx := store.WithTenantID(store.WithAgentID(context.Background(), uuid.New()), uuid.New()) + result := tool.Execute(ctx, map[string]any{"command": "git push -u origin feature"}) + + if !result.IsError { + t.Fatalf("expected missing typed git credential to fail") + } + if !strings.Contains(result.ForLLM, "Git credential resolution failed") { + t.Fatalf("expected git credential diagnostic, got: %s", result.ForLLM) + } + if strings.Contains(result.ForLLM, "raw git reached") { + t.Fatalf("raw git command executed before credential resolution failed: %s", result.ForLLM) + } +}