harden: credential redaction, error handling, RBAC scoping, tests

cmd/create_test.go

@@ -0,0 +1,140 @@
+package cmd
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/robbycochran/harness-openshell/internal/gateway"
+	"github.com/robbycochran/harness-openshell/internal/preflight"
+	"github.com/robbycochran/harness-openshell/internal/profile"
+)
+
+func TestActiveGatewayInfo_ListError(t *testing.T) {
+	gw := &mockGW{}
+
+	_, err := activeGatewayInfo(gw)
+	if err == nil {
+		t.Fatal("expected error when no active gateway")
+	}
+	if !strings.Contains(err.Error(), "no active gateway") {
+		t.Errorf("error = %q, want 'no active gateway'", err)
+	}
+}
+
+func TestActiveGatewayInfo_RemoteGateway(t *testing.T) {
+	gw := &mockGW{
+		gatewayListResult: []gateway.GatewayInfo{
+			{Name: "openshell-remote-ocp", Endpoint: "https://gateway.apps.ocp.example.com:443", Active: true},
+		},
+	}
+
+	info, err := activeGatewayInfo(gw)
+	if err != nil {
+		t.Fatalf("activeGatewayInfo: %v", err)
+	}
+	if info.Name != "openshell-remote-ocp" {
+		t.Errorf("Name = %q, want openshell-remote-ocp", info.Name)
+	}
+}
+
+func TestCreateDirect_NoProviders(t *testing.T) {
+	dir := setupTestProfile(t)
+	gw := &mockGW{
+		providers: map[string]bool{},
+	}
+
+	cfg, err := profile.Parse(dir, "default")
+	if err != nil {
+		t.Fatalf("parse profile: %v", err)
+	}
+
+	err = createDirect(dir, gw, "default", cfg, nil)
+	if err != nil {
+		t.Fatalf("createDirect: %v", err)
+	}
+	if gw.createCalls != 1 {
+		t.Errorf("createCalls = %d, want 1", gw.createCalls)
+	}
+	opts := gw.createOpts[0]
+	if len(opts.Providers) != 0 {
+		t.Errorf("Providers = %v, want empty", opts.Providers)
+	}
+}
+
+func TestCreateDirect_WithProviders(t *testing.T) {
+	dir := setupTestProfile(t)
+	gw := &mockGW{
+		providers: map[string]bool{"github": true},
+	}
+
+	cfg, err := profile.Parse(dir, "default")
+	if err != nil {
+		t.Fatalf("parse profile: %v", err)
+	}
+
+	err = createDirect(dir, gw, "default", cfg, []string{"github"})
+	if err != nil {
+		t.Fatalf("createDirect: %v", err)
+	}
+	opts := gw.createOpts[0]
+	if len(opts.Providers) != 1 || opts.Providers[0] != "github" {
+		t.Errorf("Providers = %v, want [github]", opts.Providers)
+	}
+	if opts.TTY {
+		t.Error("TTY should be false for create (non-interactive)")
+	}
+}
+
+func TestCreateDirect_SandboxName(t *testing.T) {
+	dir := setupTestProfile(t)
+	gw := &mockGW{
+		providers: map[string]bool{},
+	}
+
+	cfg, err := profile.Parse(dir, "default")
+	if err != nil {
+		t.Fatalf("parse profile: %v", err)
+	}
+	cfg.Name = "custom-sandbox"
+
+	err = createDirect(dir, gw, "default", cfg, nil)
+	if err != nil {
+		t.Fatalf("createDirect: %v", err)
+	}
+	opts := gw.createOpts[0]
+	if opts.Name != "custom-sandbox" {
+		t.Errorf("Name = %q, want custom-sandbox", opts.Name)
+	}
+}
+
+func TestProfileHasCustomProviders_NoCustom(t *testing.T) {
+	allProviders := []preflight.Provider{
+		{Name: "github", Type: "openshell"},
+		{Name: "vertex-local", Type: "openshell"},
+	}
+	if profileHasCustomProviders([]string{"github", "vertex-local"}, allProviders) {
+		t.Error("no custom providers, should return false")
+	}
+}
+
+func TestProfileHasCustomProviders_WithCustom(t *testing.T) {
+	allProviders := []preflight.Provider{
+		{Name: "github", Type: "openshell"},
+		{Name: "gws", Type: "custom"},
+	}
+	if !profileHasCustomProviders([]string{"github", "gws"}, allProviders) {
+		t.Error("gws is custom, should return true")
+	}
+}
+
+func TestProviderInList(t *testing.T) {
+	if !providerInList("github", []string{"github", "vertex-local"}) {
+		t.Error("github should be in list")
+	}
+	if providerInList("atlassian", []string{"github", "vertex-local"}) {
+		t.Error("atlassian should not be in list")
+	}
+	if providerInList("github", nil) {
+		t.Error("nil list should return false")
+	}
+}

cmd/deploy.go

@@ -181,9 +181,8 @@ func deployFromConfig(harnessDir string, gwCfg *gateway.GatewayConfig, gw gatewa
 		for _, sa := range gwCfg.OCP.SCCAnyuid {
 			kc.RunOC(ctx, "adm", "policy", "add-scc-to-user", "anyuid", "-z", sa, "-n", namespace)
 		}
-		clusterRunner.RunKubectl(ctx, "create", "clusterrolebinding", "agent-sandbox-admin",
-			"--clusterrole=cluster-admin",
-			"--serviceaccount=agent-sandbox-system:agent-sandbox-controller")
+		// The sandbox controller ClusterRole and ClusterRoleBinding are
+		// included in the upstream manifest.yaml applied in step 2.
 	}
 
 	// Addon manifests (RBAC, etc.)
@@ -249,7 +248,10 @@ func deployFromConfig(harnessDir string, gwCfg *gateway.GatewayConfig, gw gatewa
 		return fmt.Errorf("service type %q endpoint resolution not yet implemented", gwCfg.Gateway.Service)
 	}
 
-	existing, _ := gw.GatewayList()
+	existing, err := gw.GatewayList()
+	if err != nil {
+		return fmt.Errorf("listing existing gateways: %w", err)
+	}
 	for _, g := range existing {
 		if routeHost != "" && strings.Contains(g.Endpoint, routeHost) {
 			gw.GatewayRemove(g.Name)

cmd/providers.go

@@ -53,7 +53,10 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool, gwCfg
 
 	// Force mode: require no running sandboxes
 	if force {
-		sandboxes, _ := gw.SandboxList()
+		sandboxes, err := gw.SandboxList()
+		if err != nil {
+			return fmt.Errorf("listing sandboxes: %w", err)
+		}
 		if len(sandboxes) > 0 {
 			return fmt.Errorf("cannot --force with running sandboxes — delete them first")
 		}
@@ -94,7 +97,10 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool, gwCfg
 
 	// Show results
 	status.Section("Providers")
-	names, _ := gw.ProviderList()
+	names, err := gw.ProviderList()
+	if err != nil {
+		return fmt.Errorf("listing providers: %w", err)
+	}
 	for _, n := range names {
 		status.OK(n)
 	}

cmd/providers_test.go

@@ -0,0 +1,142 @@
+package cmd
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/robbycochran/harness-openshell/internal/gateway"
+)
+
+func setupProvidersTest(t *testing.T) string {
+	t.Helper()
+	dir := t.TempDir()
+	os.MkdirAll(filepath.Join(dir, "sandbox", "profiles"), 0o755)
+	return dir
+}
+
+func TestRegisterProviders_GitHubWhenTokenSet(t *testing.T) {
+	dir := setupProvidersTest(t)
+	t.Setenv("GITHUB_TOKEN", "ghp_test123")
+	t.Setenv("JIRA_API_TOKEN", "")
+
+	gw := &mockGW{
+		providers: map[string]bool{},
+	}
+
+	err := registerProviders(dir, gw, false, nil)
+	if err != nil {
+		t.Fatalf("registerProviders: %v", err)
+	}
+}
+
+func TestRegisterProviders_SkipsWhenTokenMissing(t *testing.T) {
+	dir := setupProvidersTest(t)
+	t.Setenv("GITHUB_TOKEN", "")
+	t.Setenv("JIRA_API_TOKEN", "")
+
+	gw := &mockGW{
+		providers: map[string]bool{},
+	}
+
+	err := registerProviders(dir, gw, false, nil)
+	if err != nil {
+		t.Fatalf("registerProviders: %v", err)
+	}
+}
+
+func TestRegisterProviders_SkipsExistingProvider(t *testing.T) {
+	dir := setupProvidersTest(t)
+	t.Setenv("GITHUB_TOKEN", "ghp_test123")
+	t.Setenv("JIRA_API_TOKEN", "")
+
+	gw := &mockGW{
+		providers: map[string]bool{"github": true},
+	}
+
+	err := registerProviders(dir, gw, false, nil)
+	if err != nil {
+		t.Fatalf("registerProviders: %v", err)
+	}
+}
+
+func TestRegisterProviders_ForceWithRunningSandboxes(t *testing.T) {
+	dir := setupProvidersTest(t)
+
+	gw := &mockGWWithSandboxes{
+		mockGW: &mockGW{
+			providers: map[string]bool{"github": true},
+		},
+		sandboxes: []string{"test-sandbox"},
+	}
+
+	err := registerProviders(dir, gw, true, nil)
+	if err == nil {
+		t.Fatal("expected error with --force and running sandboxes")
+	}
+	if !strings.Contains(err.Error(), "cannot --force") {
+		t.Errorf("error = %q, want 'cannot --force'", err)
+	}
+}
+
+func TestRegisterProviders_ForceDeletesAndRecreates(t *testing.T) {
+	dir := setupProvidersTest(t)
+	t.Setenv("GITHUB_TOKEN", "ghp_test123")
+	t.Setenv("JIRA_API_TOKEN", "")
+
+	gw := &mockGW{
+		providers: map[string]bool{},
+	}
+
+	err := registerProviders(dir, gw, true, nil)
+	if err != nil {
+		t.Fatalf("registerProviders: %v", err)
+	}
+}
+
+func TestRegisterProviders_RespectsGatewayConfig(t *testing.T) {
+	dir := setupProvidersTest(t)
+	t.Setenv("GITHUB_TOKEN", "ghp_test123")
+	t.Setenv("JIRA_API_TOKEN", "token")
+
+	gw := &mockGW{
+		providers: map[string]bool{},
+	}
+
+	gwCfg := &gateway.GatewayConfig{}
+	gwCfg.Providers.Enabled = []string{"github"}
+
+	err := registerProviders(dir, gw, false, gwCfg)
+	if err != nil {
+		t.Fatalf("registerProviders: %v", err)
+	}
+}
+
+func TestRegisterProviders_ListError(t *testing.T) {
+	dir := setupProvidersTest(t)
+
+	gw := &mockGW{
+		providers:   map[string]bool{},
+		providerErr: fmt.Errorf("gateway unreachable"),
+	}
+
+	err := registerProviders(dir, gw, false, nil)
+	if err == nil {
+		t.Fatal("expected error when provider list fails")
+	}
+	if !strings.Contains(err.Error(), "listing providers") {
+		t.Errorf("error = %q, want 'listing providers'", err)
+	}
+}
+
+// mockGWWithSandboxes wraps mockGW to return a non-empty sandbox list.
+type mockGWWithSandboxes struct {
+	*mockGW
+	sandboxes []string
+}
+
+func (m *mockGWWithSandboxes) SandboxList() ([]string, error) {
+	return m.sandboxes, nil
+}

cmd/teardown.go

@@ -108,7 +108,10 @@ func teardownProviders(gw gateway.Gateway, activeGW string) error {
 	if len(remaining) > 0 {
 		// Sandbox may be mid-deletion — wait briefly and retry
 		time.Sleep(2 * time.Second)
-		remaining, _ = gw.SandboxList()
+		remaining, err = gw.SandboxList()
+		if err != nil {
+			return fmt.Errorf("rechecking sandboxes: %w", err)
+		}
 		if len(remaining) > 0 {
 			return fmt.Errorf("cannot delete providers with running sandboxes — run: harness teardown --sandboxes")
 		}
@@ -191,6 +194,7 @@ func teardownK8s(gw gateway.Gateway, gwCfg *gateway.GatewayConfig, kc, clusterRu
 	for _, sa := range sccAnyuid {
 		kc.RunOC(ctx, "adm", "policy", "remove-scc-from-user", "anyuid", "-z", sa, "-n", namespace)
 	}
+	// Clean up legacy cluster-admin binding (no longer created, but may exist from earlier deploys)
 	clusterRunner.RunKubectl(ctx, "delete", "clusterrolebinding", "agent-sandbox-admin")
 	status.Info("Cleared")
 
@@ -217,7 +221,11 @@ func teardownK8s(gw gateway.Gateway, gwCfg *gateway.GatewayConfig, kc, clusterRu
 	// Gateway config cleanup
 	fmt.Println()
 	status.Section("Gateway config")
-	gateways, _ := gw.GatewayList()
+	gateways, err := gw.GatewayList()
+	if err != nil {
+		status.Failf("listing gateways: %v", err)
+		return
+	}
 	for _, g := range gateways {
 		if !strings.Contains(g.Endpoint, "127.0.0.1") {
 			if err := gw.GatewayRemove(g.Name); err == nil {