Files
kandev/apps/backend/internal/agentctl/server/config/config_test.go
T
João Salavisa 4d1787f484 fix: harden agentctl bind, git clone args, and powershell sound escaping (#1811)
* fix: harden agentctl bind, git clone args, and powershell sound escaping

Three LOW-severity defense-in-depth hardening fixes with PoC + regression tests:

- notifications: escapePowerShell now neutralizes backtick and `$` in addition
  to `"`, defusing `$(...)` sub-expression and `$var` injection through an
  operator-supplied SoundFile path interpolated into the powershell `-c` script.
- office/skills: git clone for git-sourced skills now passes a `--`
  end-of-options separator before the source locator (matching configloader),
  so a `-`-prefixed locator can never be parsed as a git flag.
- agentctl: new Config.ListenHost() binds the control server and per-instance
  listeners to loopback only when no AuthToken is configured (auth disabled),
  so the unauthenticated command/shell/process routes are never reachable
  beyond localhost. The normal nonce-handshake flow (incl. Docker) always has
  a token and keeps binding all interfaces.

* test: make PoC tests flip and drop indexOf coupling

Address review feedback on the security-hardening PoCs:

- system_provider PoC now demonstrates the PowerShell sub-expression injection
  against a legacyEscapePowerShell copy of the unfixed function and asserts the
  current escaper defuses it, so the PoC genuinely flips instead of passing
  trivially against fixed code (a substring match on `$(` matched the escaped
  form too).
- materialization PoC + regression test use stdlib slices.Index instead of a
  custom indexOf defined in the PoC file, removing the coupling where deleting
  the PoC file would break the regression test.

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
2026-07-20 11:32:43 +01:00

165 lines
5.3 KiB
Go

package config
import "testing"
func TestConsumeNonce(t *testing.T) {
t.Run("valid nonce returns token and burns nonce", func(t *testing.T) {
cfg := &Config{
AuthToken: "test-token-123",
BootstrapNonce: "nonce-abc",
}
token := cfg.ConsumeNonce("nonce-abc")
if token != "test-token-123" {
t.Fatalf("expected test-token-123, got %q", token)
}
// Second call should fail (nonce burned)
token2 := cfg.ConsumeNonce("nonce-abc")
if token2 != "" {
t.Fatalf("expected empty after nonce burn, got %q", token2)
}
})
t.Run("wrong nonce returns empty", func(t *testing.T) {
cfg := &Config{
AuthToken: "test-token",
BootstrapNonce: "correct-nonce",
}
token := cfg.ConsumeNonce("wrong-nonce")
if token != "" {
t.Fatalf("expected empty for wrong nonce, got %q", token)
}
// Original nonce should still work
token2 := cfg.ConsumeNonce("correct-nonce")
if token2 != "test-token" {
t.Fatalf("expected test-token, got %q", token2)
}
})
t.Run("empty nonce returns empty", func(t *testing.T) {
cfg := &Config{
AuthToken: "test-token",
BootstrapNonce: "some-nonce",
}
token := cfg.ConsumeNonce("")
if token != "" {
t.Fatalf("expected empty for empty nonce, got %q", token)
}
})
t.Run("no bootstrap nonce configured returns empty", func(t *testing.T) {
cfg := &Config{
AuthToken: "test-token",
}
token := cfg.ConsumeNonce("any-nonce")
if token != "" {
t.Fatalf("expected empty when no nonce configured, got %q", token)
}
})
}
func TestGenerateSelfToken(t *testing.T) {
token := generateSelfToken()
if len(token) != 64 { // 32 bytes hex-encoded = 64 chars
t.Fatalf("expected 64-char hex token, got %d chars: %q", len(token), token)
}
// Tokens should be unique
token2 := generateSelfToken()
if token == token2 {
t.Fatal("expected unique tokens, got identical")
}
}
// TestInjectKandevMcpServer_HttpFirst is the McpServerConfig counterpart to
// api.TestInjectKandevMcpServers_HttpFirst. HTTP must appear before SSE so the
// downstream capability filter dedup keeps the HTTP transport for agents that
// advertise both transports.
func TestInjectKandevMcpServer_HttpFirst(t *testing.T) {
got := injectKandevMcpServer(nil, 12345)
if len(got) != 2 {
t.Fatalf("expected 2 kandev entries, got %d: %+v", len(got), got)
}
if got[0].Name != kandevMcpServerName || got[0].Type != "http" {
t.Errorf("expected first entry name=%q type=http, got name=%q type=%q",
kandevMcpServerName, got[0].Name, got[0].Type)
}
if got[0].URL != "http://localhost:12345/mcp" {
t.Errorf("expected http URL .../mcp, got %q", got[0].URL)
}
if got[1].Name != kandevMcpServerName || got[1].Type != "sse" {
t.Errorf("expected second entry name=%q type=sse, got name=%q type=%q",
kandevMcpServerName, got[1].Name, got[1].Type)
}
if got[1].URL != "http://localhost:12345/sse" {
t.Errorf("expected sse URL .../sse, got %q", got[1].URL)
}
upstream := []McpServerConfig{
{Name: "other", Type: "stdio", Command: "x"},
{Name: kandevMcpServerName, Type: "sse", URL: "http://stale/sse"},
}
got = injectKandevMcpServer(upstream, 12345)
if len(got) != 3 {
t.Fatalf("expected 3 entries (http+sse+other), got %d: %+v", len(got), got)
}
if got[0].Type != "http" || got[1].Type != "sse" {
t.Errorf("expected injected order http,sse; got %q,%q", got[0].Type, got[1].Type)
}
if got[2].Name != "other" || got[2].Command != "x" {
t.Errorf("expected upstream 'other' entry last, got %+v", got[2])
}
}
// TestApplyOverrides_BaseBranches verifies the per-instance per-repo base
// branch map flows from the HTTP-request-driven InstanceOverrides onto the
// InstanceConfig the process manager consumes. Empty/nil overrides must NOT
// clobber a previously-set value — applyOverrides is a one-shot merge.
func TestApplyOverrides_BaseBranches(t *testing.T) {
t.Run("non-empty overrides populate cfg", func(t *testing.T) {
cfg := &InstanceConfig{}
applyOverrides(cfg, &InstanceOverrides{
BaseBranches: map[string]string{"repo-a": "main", "repo-b": "develop"},
})
if got := cfg.BaseBranches["repo-a"]; got != "main" {
t.Errorf("BaseBranches[repo-a] = %q, want main", got)
}
if got := cfg.BaseBranches["repo-b"]; got != "develop" {
t.Errorf("BaseBranches[repo-b] = %q, want develop", got)
}
})
t.Run("empty overrides preserve existing", func(t *testing.T) {
cfg := &InstanceConfig{
BaseBranches: map[string]string{"repo-a": "main"},
}
applyOverrides(cfg, &InstanceOverrides{BaseBranches: nil})
if got := cfg.BaseBranches["repo-a"]; got != "main" {
t.Errorf("empty overrides clobbered existing value; got %q", got)
}
})
}
// TestListenHost pins the loopback-only bind guard: when auth is disabled
// (empty AuthToken) the server must bind loopback only; when a token is
// configured it binds all interfaces (empty host → ":port").
func TestListenHost(t *testing.T) {
t.Run("no token binds loopback only", func(t *testing.T) {
cfg := &Config{AuthToken: ""}
if got := cfg.ListenHost(); got != "127.0.0.1" {
t.Fatalf("ListenHost() = %q, want 127.0.0.1 when auth disabled", got)
}
})
t.Run("token binds all interfaces", func(t *testing.T) {
cfg := &Config{AuthToken: "secret"}
if got := cfg.ListenHost(); got != "" {
t.Fatalf("ListenHost() = %q, want \"\" (all interfaces) when auth enabled", got)
}
})
}