Files
goclaw/internal/tools/credential_adapter_git_ssh_test.go
T
Duy /zuey/andGitHub a591473546 feat(secure-cli): CLI credential adapters framework + git adapter (#82) (#89)
* feat(secure-cli): Phase 1 schema + storage delta (issue #82)

Adds `adapter_name` column to secure_cli_binaries and `credential_type` + `host_scope` columns to secure_cli_user_credentials. LookupByBinary LEFT JOIN now projects AdapterName, UserCredentialType, and UserHostScope. Extends SecureCLIStore with SetUserCredentialsTyped(ctx, binaryID, userID, env, credType, hostScope); legacy SetUserCredentials delegates to typed variant with nil/nil for backward compat.

PG migration 73 + RequiredSchemaVersion bumped to 73. SQLite incremental migrations (versions 39–41) + SchemaVersion 42. 7 SQLite + 6 PG integration tests covering schema, round-trip, NULL legacy, LookupByBinary projection.

Fixes #82

* feat(secure-cli): Phase 2 CredentialAdapter framework + passthrough (issue #82)

Adds CredentialAdapter interface + Injection{ArgvPrefix,Env,Cleanup,ScrubValues} struct. Registry resolves by name; falls back to passthrough for empty/unknown. passthroughAdapter is default no-op — all existing presets (gh/aws/gcloud/kubectl/terraform/gws) behave bit-for-bit identically.

Per-request WithScrubBag(ctx) + AddScrubValuesCtx + ScrubCredentialsCtx for multi-tenant secret isolation (replaces package-global slice). Hook in executeCredentialed between env merge and exec: resolve adapter by bin.AdapterName, reject non-passthrough in sandbox, call Prepare, splice ArgvPrefix, merge Env, defer Cleanup, register ScrubValues.

Audit log security.system_env_injection records adapter name + env key names + argv_prefix_len + sha256(host_scope) — NEVER values. CLIPreset.AdapterName field added; empty default for legacy presets.

12 unit tests covering passthrough no-op, registry fallback + nil safety, Injection shape, hashHostScope determinism, sortedKeys, scrub bag per-request isolation + concurrent + short-value guard.

Fixes #82

* feat(secure-cli): Phase 2b extensibility helpers + psql stub adapter (issue #82)

Proves CredentialAdapter framework generalizes beyond git. Adds materializeEphemeral(ctx, content, prefix) shared helper — 0600 tmpfile + idempotent atomic.Bool cleanup latch; memfd intentionally rejected (resolves "self" against child process).

Adds psqlAdapter consuming framework end-to-end via PGPASSFILE pattern; libpq-spec .pgpass escaping for `:` and `\`. Registers psql preset with AdapterName: "psql" (production UI for typed creds lands in v2).

Interface validation gate passes: Injection shape unchanged, hook is psql-agnostic, no special branch needed.

Tests: ephemeral write/cleanup/concurrent/zero-content; psql routing/content/escaping/error paths/registration.

Fixes #82

* feat(secure-cli): git adapter PAT + SSH implementation (issue #82)

Phase 3 — PAT path
- gitAdapter PAT branch via GIT_CONFIG_COUNT/KEY_0/VALUE_0 env (git 2.31+)
  so the token never lands on argv, .git/config, or remote URL
- Host-scope enforcement with IDN normalization (golang.org/x/net/idna)
  and embedded-userinfo rejection in URL parsing
- CVE-2018-17456 mitigation: resolve remote URLs via `git config --get`
  not `git remote get-url` to dodge ext::sh protocol handler injection
- Case-insensitive DenyArgs blocking `-c http.`, `-c credential.`,
  `-c core.sshcommand`, `config --global/--system`, `credential-helper`,
  bare `daemon`
- New WithExecCwd / ExecCwdFromContext context helpers — fixes a latent
  design gap where the adapter's pre-flight `git config --get` ran in
  goclaw's daemon CWD instead of the agent's repo
- 14 unit tests covering subcommand routing, host normalization,
  scp-form parsing, userinfo rejection, CRLF token rejection,
  CVE-2018-17456 regression, DenyArgs preset coverage
- 3 integration tests against a local TLS git-http-backend server
  proving end-to-end clone + fetch + host-mismatch rejection with
  zero token leakage into the cloned .git/config

Phase 4 — SSH path
- gitAdapter ssh_key branch materializes per-call 0600 tmpfile via
  the Phase 2b materializeEphemeral helper, injects GIT_SSH_COMMAND
  with -o IdentitiesOnly=yes -o BatchMode=yes
  -o StrictHostKeyChecking=accept-new, idempotent cleanup
- ValidateSSHKey using golang.org/x/crypto/ssh rejects
  passphrase-protected keys via ErrSSHKeyPassphraseUnsupported sentinel
- 8 unit tests covering passphrase rejection, env shape, cleanup
  lifecycle, host-mismatch reuse, malformed-blob rejection
- 3 integration tests proving tmpfile 0600 lifecycle, env propagation
  to child process, cleanup-on-exec-failure, no-orphan-on-rejection
- 2 new i18n keys (en/vi/zh) for SSH-passphrase and SSH-key-invalid

Verification: go vet clean, go build ./... clean,
go build -tags sqliteonly ./... clean, go test -race
./internal/tools/ pass, go test -tags integration
./tests/integration/ -run TestGitAdapter pass.

Refs #82

* feat(secure-cli): UI presets + typed credential HTTP path + i18n (issue #82)

Phase 5. Typed PUT envelope with {error:{code,message}, error_key} for
field-level errors. CliCredentialGitFields React component (PAT/SSH picker,
host_scope input, CRLF→LF normalization, masked-edit). 17 i18n keys × 3
locales (backend + frontend). i18n parity test.

* feat(secure-cli): audit log schema + adapter framework docs (issue #82)

Phase 6. emitSystemEnvInjectionAudit helper centralizes
security.system_env_injection slog with host_scope_hash (SHA-256 8 hex,
plaintext hostname omitted for PII safety). Audit shape pinned by
TestEmitSystemEnvInjectionAudit_*. New docs: git-credential-adapter.md
(user guide), credential-adapter-playbook.md (R1 implementer guide w/
kubectl/docker/npm/aws/psql worked mappings). 09-security.md § 14
trust-boundary diagram + SSH TOFU + SIGKILL caveats. 03-tools-system.md
§ 8a. Changelog entry.

* docs(journal): issue #82 CLI credential adapters shipped

Retrospective covering 6 commits across phases 1-6: framework, git PAT/SSH, psql stub, UI/i18n, audit log schema, docs. Notes memfd-drop rationale, TOFU/SIGKILL caveats, sentinel-length lesson from audit-shape tests.

* fix(secure-cli): honor per-request scrub bag on success+failure paths (#82)

Sandbox and host exec paths called the non-Ctx ScrubCredentials, so
adapter ScrubValues registered into the per-request bag during Prepare
(e.g. GitLab glpat-, Bitbucket app-passwords, Azure DevOps PATs, Gitea
tokens, SSH key tmpfile paths) were ignored on stdout/stderr returned
to the agent. Only the package-global regex pass ran — covering ghp_
but nothing else.

Switch all four exec/sandbox call sites to ScrubCredentialsCtx so the
bag is consulted. Also scrub the slog adapter_cleanup_failed line —
os.Remove errors embed the full tmpfile path.

Add 5 regression tests that pin success path, failure path, negative
control (proves the bag is what catches the sentinel), classic-PAT
sanity, and timeout path no-leak.

Locks AC6 against non-GitHub PAT providers.

* fix(secure-cli): address review-pr #89 findings

- ui/web: SSH key Textarea was 'text-xs' on all viewports, triggering
  iOS Safari auto-zoom on focus. Switch to 'text-base md:text-xs' so
  mobile renders 16px (no zoom) while desktop keeps compact mono font.
- psql adapter: add on-disk pgpass tmpfile path to ScrubValues. psql
  echoes 'could not open password file "<path>"' on IO errors, so
  the path needs scrubbing alongside the password. Mirrors the git SSH
  adapter pattern. Update test assertion accordingly.
- psql adapter: replace literal 'nil' context with 'context.TODO()'
  to silence staticcheck SA1012.
2026-05-28 18:17:49 +07:00

241 lines
6.8 KiB
Go

package tools
// Phase 4 unit tests for gitAdapter's ssh_key branch and ValidateSSHKey.
// Integration coverage (real exec, tmpfile lifecycle under a child process)
// lives in tests/integration/git_adapter_ssh_test.go.
import (
"context"
"crypto/ed25519"
"crypto/rand"
"encoding/base64"
"encoding/json"
"errors"
"os"
"runtime"
"strings"
"testing"
"golang.org/x/crypto/ssh"
"github.com/nextlevelbuilder/goclaw/internal/store"
)
// genEd25519PEM returns an unencrypted OpenSSH PEM-encoded ed25519 private key.
func genEd25519PEM(t *testing.T) []byte {
t.Helper()
_, priv, err := ed25519.GenerateKey(rand.Reader)
if err != nil {
t.Fatalf("ed25519 gen: %v", err)
}
block, err := ssh.MarshalPrivateKey(priv, "")
if err != nil {
t.Fatalf("marshal key: %v", err)
}
return []byte("-----BEGIN OPENSSH PRIVATE KEY-----\n" +
base64Encode(block.Bytes) +
"\n-----END OPENSSH PRIVATE KEY-----\n")
}
// genEd25519PEMWithPassphrase returns a passphrase-encrypted ed25519 key.
func genEd25519PEMWithPassphrase(t *testing.T, pw string) []byte {
t.Helper()
_, priv, err := ed25519.GenerateKey(rand.Reader)
if err != nil {
t.Fatalf("ed25519 gen: %v", err)
}
block, err := ssh.MarshalPrivateKeyWithPassphrase(priv, "", []byte(pw))
if err != nil {
t.Fatalf("marshal encrypted key: %v", err)
}
return []byte("-----BEGIN OPENSSH PRIVATE KEY-----\n" +
base64Encode(block.Bytes) +
"\n-----END OPENSSH PRIVATE KEY-----\n")
}
// base64Encode wraps stdlib base64 at 70 cols to mimic real OpenSSH PEM.
func base64Encode(b []byte) string {
s := base64.StdEncoding.EncodeToString(b)
var out strings.Builder
for i := 0; i < len(s); i += 70 {
end := i + 70
if end > len(s) {
end = len(s)
}
if i > 0 {
out.WriteByte('\n')
}
out.WriteString(s[i:end])
}
return out.String()
}
// 1. AC5: passphrase keys rejected via sentinel.
func TestValidateSSHKey_RejectsPassphrase(t *testing.T) {
pem := genEd25519PEMWithPassphrase(t, "topsecret")
err := ValidateSSHKey(pem)
if !errors.Is(err, ErrSSHKeyPassphraseUnsupported) {
t.Fatalf("want ErrSSHKeyPassphraseUnsupported, got %v", err)
}
}
// 2. Plain ed25519 passes.
func TestValidateSSHKey_AcceptsUnencrypted(t *testing.T) {
if err := ValidateSSHKey(genEd25519PEM(t)); err != nil {
t.Fatalf("ed25519 should pass: %v", err)
}
}
// 3. Garbage rejected without exposing as passphrase.
func TestValidateSSHKey_RejectsGarbage(t *testing.T) {
err := ValidateSSHKey([]byte("not a key"))
if err == nil {
t.Fatal("expected error")
}
if errors.Is(err, ErrSSHKeyPassphraseUnsupported) {
t.Fatal("garbage must not be classified as passphrase-protected")
}
}
// 4. Empty rejected.
func TestValidateSSHKey_RejectsEmpty(t *testing.T) {
if err := ValidateSSHKey(nil); err == nil {
t.Fatal("empty must error")
}
}
// 5. Prepare(ssh_key) shape: env, options, scrub, cleanup.
func TestGitAdapter_PrepareSSH(t *testing.T) {
pem := genEd25519PEM(t)
blob, _ := json.Marshal(map[string]string{"key": string(pem)})
ct, hs := "ssh_key", "github.com"
cred := &store.SecureCLIUserCredential{
CredentialType: &ct,
HostScope: &hs,
EncryptedEnv: blob,
}
a := gitAdapter{}
inj, err := a.Prepare(context.Background(), nil, cred,
[]string{"clone", "git@github.com:o/r.git", "/tmp/dst"})
if err != nil {
t.Fatalf("Prepare: %v", err)
}
if len(inj.ArgvPrefix) != 0 {
t.Fatalf("argv leak: %v", inj.ArgvPrefix)
}
gsc := inj.Env["GIT_SSH_COMMAND"]
if !strings.HasPrefix(gsc, "ssh -i ") {
t.Fatalf("GIT_SSH_COMMAND prefix wrong: %q", gsc)
}
for _, need := range []string{
"-o IdentitiesOnly=yes",
"-o BatchMode=yes",
"-o StrictHostKeyChecking=accept-new",
} {
if !strings.Contains(gsc, need) {
t.Fatalf("GIT_SSH_COMMAND missing %q\nfull: %s", need, gsc)
}
}
// Extract keypath: token after "-i ".
rest := strings.TrimPrefix(gsc, "ssh -i ")
keyPath := strings.SplitN(rest, " ", 2)[0]
if !strings.Contains(keyPath, "goclaw-gitkey-") {
t.Fatalf("keypath not in expected tmp prefix: %s", keyPath)
}
if !strings.HasPrefix(keyPath, os.TempDir()) {
t.Fatalf("keypath not under TempDir(%s): %s", os.TempDir(), keyPath)
}
// 0600 perms (POSIX only).
if runtime.GOOS != "windows" {
st, err := os.Stat(keyPath)
if err != nil {
t.Fatalf("stat keypath: %v", err)
}
if st.Mode().Perm() != 0o600 {
t.Fatalf("keypath perms = %o, want 0600", st.Mode().Perm())
}
}
if len(inj.ScrubValues) != 1 || inj.ScrubValues[0] != keyPath {
t.Fatalf("ScrubValues = %v, want [%s]", inj.ScrubValues, keyPath)
}
if inj.Cleanup == nil {
t.Fatal("Cleanup must be non-nil")
}
// File present pre-cleanup, gone after.
if _, err := os.Stat(keyPath); err != nil {
t.Fatalf("pre-cleanup stat: %v", err)
}
if err := inj.Cleanup(); err != nil {
t.Fatalf("cleanup: %v", err)
}
if _, err := os.Stat(keyPath); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("post-cleanup stat want ErrNotExist, got: %v", err)
}
}
// 6. SSH path also enforces host match: scp-form mismatch.
func TestGitAdapter_PrepareSSH_HostMismatchSCPForm(t *testing.T) {
pem := genEd25519PEM(t)
blob, _ := json.Marshal(map[string]string{"key": string(pem)})
ct, hs := "ssh_key", "github.com"
cred := &store.SecureCLIUserCredential{
CredentialType: &ct,
HostScope: &hs,
EncryptedEnv: blob,
}
a := gitAdapter{}
_, err := a.Prepare(context.Background(), nil, cred,
[]string{"clone", "git@gitlab.com:o/r.git"})
if err == nil {
t.Fatal("expected host mismatch")
}
var mismatch *errCredentialHostMismatch
if !errors.As(err, &mismatch) {
t.Fatalf("want *errCredentialHostMismatch, got %T: %v", err, err)
}
if strings.Contains(err.Error(), string(pem)) {
t.Fatal("key PEM leaked into error message")
}
}
// 7. Cleanup is idempotent — Phase 2b lockstep test.
func TestGitAdapter_PrepareSSH_CleanupIdempotent(t *testing.T) {
pem := genEd25519PEM(t)
blob, _ := json.Marshal(map[string]string{"key": string(pem)})
ct, hs := "ssh_key", "github.com"
cred := &store.SecureCLIUserCredential{
CredentialType: &ct,
HostScope: &hs,
EncryptedEnv: blob,
}
a := gitAdapter{}
inj, err := a.Prepare(context.Background(), nil, cred,
[]string{"clone", "git@github.com:o/r.git"})
if err != nil {
t.Fatalf("Prepare: %v", err)
}
if err := inj.Cleanup(); err != nil {
t.Fatalf("first cleanup: %v", err)
}
if err := inj.Cleanup(); err != nil {
t.Fatalf("second cleanup must be nil, got %v", err)
}
}
// 8. Blob missing 'key' field rejected before any tmpfile is created.
func TestGitAdapter_PrepareSSH_RejectsMalformedBlob(t *testing.T) {
ct, hs := "ssh_key", "github.com"
cred := &store.SecureCLIUserCredential{
CredentialType: &ct,
HostScope: &hs,
EncryptedEnv: []byte(`{"private_key":"-----BEGIN..."}`),
}
a := gitAdapter{}
_, err := a.Prepare(context.Background(), nil, cred,
[]string{"clone", "git@github.com:o/r.git"})
if err == nil {
t.Fatal("expected error for missing 'key' field")
}
}