fix(security,api-server,control-plane): CodeRabbit fixes, deletecollection fallback, idempotent agent start

.claude/skills/ambient-pr-test/SKILL.md

@@ -244,6 +244,14 @@ docker login quay.io
 
 Either `build.sh` was not run or the CI build workflow failed. Check Actions → `Build and Push Component Docker Images` for the PR.
 
+### Runner pods can't reach external hosts (Squid proxy)
+
+The MPP cluster routes outbound traffic through a Squid proxy (`proxy.squi-001.prod.iad2.dc.redhat.com:3128`). The `runtime-int` deployments have `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` set in their pod specs, but runner pods spawned by the control plane did not inherit these.
+
+**Fix (merged):** The control plane reads `HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY` from its own environment and injects them into both the runner container (`buildEnv()`) and the MCP sidecar container (`buildMCPSidecar()`). No manifest change needed — the CP's deployment already has the proxy vars; they now propagate automatically.
+
+**Pattern:** When the CP needs to forward platform-level env vars to spawned pods, add the field to `ControlPlaneConfig` → `KubeReconcilerConfig` → `buildEnv()`/`buildMCPSidecar()`.
+
 ### JWT / UNAUTHENTICATED errors in api-server
 
 The production overlay configures JWT against Red Hat SSO. For ephemeral test instances without SSO integration:

.claude/skills/devflow/SKILL.md

@@ -31,7 +31,7 @@ Then re-read this file before continuing.
 
 ## Workflow Overview
 
-```
+```text
 1. Ticket (optional)
 2. Branch (from ticket name if available)
 3. Spec change (load + modify the component's .spec.md)

components/ambient-api-server/plugins/agents/start_handler.go

@@ -47,16 +47,27 @@ func NewStartHandler(agent AgentService, inboxSvc inbox.InboxMessageService, ses
 }
 
 func (h *startHandler) Start(w http.ResponseWriter, r *http.Request) {
-	cfg := &handlers.HandlerConfig{
-		Action: func() (interface{}, *pkgerrors.ServiceError) {
-			ctx := r.Context()
-			agentID := mux.Vars(r)["agent_id"]
+	ctx := r.Context()
+	agentID := mux.Vars(r)["agent_id"]
 
-			agent, err := h.agent.Get(ctx, agentID)
-			if err != nil {
-				return nil, err
-			}
+	agent, err := h.agent.Get(ctx, agentID)
+	if err != nil {
+		handlers.HandleError(ctx, w, err)
+		return
+	}
 
+	if existing, _ := h.session.ActiveByAgentID(ctx, agentID); existing != nil {
+		resp := &StartResponse{
+			Session: sessions.PresentSession(existing),
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		json.NewEncoder(w).Encode(resp)
+		return
+	}
+
+	cfg := &handlers.HandlerConfig{
+		Action: func() (interface{}, *pkgerrors.ServiceError) {
 			unread, inboxErr := h.inbox.UnreadByAgentID(ctx, agentID)
 			if inboxErr != nil {
 				return nil, inboxErr

components/ambient-api-server/plugins/credentials/handler.go

@@ -3,6 +3,7 @@ package credentials
 import (
 	"fmt"
 	"net/http"
+	"regexp"
 
 	"github.com/gorilla/mux"
 
@@ -13,6 +14,8 @@ import (
 	"github.com/openshift-online/rh-trex-ai/pkg/services"
 )
 
+var safeProjectIDPattern = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)
+
 var _ handlers.RestHandler = credentialHandler{}
 
 type credentialHandler struct {
@@ -58,11 +61,15 @@ func (h credentialHandler) Patch(w http.ResponseWriter, r *http.Request) {
 		Validators: []handlers.Validate{},
 		Action: func() (interface{}, *errors.ServiceError) {
 			ctx := r.Context()
+			projectID := mux.Vars(r)["id"]
 			id := mux.Vars(r)["cred_id"]
 			found, err := h.credential.Get(ctx, id)
 			if err != nil {
 				return nil, err
 			}
+			if found.ProjectID != projectID {
+				return nil, errors.NotFound("credential with id='%s' not found", id)
+			}
 
 			if patch.Name != nil {
 				found.Name = *patch.Name
@@ -106,6 +113,9 @@ func (h credentialHandler) List(w http.ResponseWriter, r *http.Request) {
 		Action: func() (interface{}, *errors.ServiceError) {
 			ctx := r.Context()
 			projectID := mux.Vars(r)["id"]
+			if !safeProjectIDPattern.MatchString(projectID) {
+				return nil, errors.Validation("invalid project ID format")
+			}
 
 			listArgs := services.NewListArguments(r.URL.Query())
 			projectFilter := fmt.Sprintf("project_id = '%s'", projectID)
@@ -148,12 +158,16 @@ func (h credentialHandler) List(w http.ResponseWriter, r *http.Request) {
 func (h credentialHandler) Get(w http.ResponseWriter, r *http.Request) {
 	cfg := &handlers.HandlerConfig{
 		Action: func() (interface{}, *errors.ServiceError) {
+			projectID := mux.Vars(r)["id"]
 			id := mux.Vars(r)["cred_id"]
 			ctx := r.Context()
 			credential, err := h.credential.Get(ctx, id)
 			if err != nil {
 				return nil, err
 			}
+			if credential.ProjectID != projectID {
+				return nil, errors.NotFound("credential with id='%s' not found", id)
+			}
 
 			return PresentCredential(credential), nil
 		},
@@ -165,9 +179,17 @@ func (h credentialHandler) Get(w http.ResponseWriter, r *http.Request) {
 func (h credentialHandler) Delete(w http.ResponseWriter, r *http.Request) {
 	cfg := &handlers.HandlerConfig{
 		Action: func() (interface{}, *errors.ServiceError) {
+			projectID := mux.Vars(r)["id"]
 			id := mux.Vars(r)["cred_id"]
 			ctx := r.Context()
-			err := h.credential.Delete(ctx, id)
+			found, err := h.credential.Get(ctx, id)
+			if err != nil {
+				return nil, err
+			}
+			if found.ProjectID != projectID {
+				return nil, errors.NotFound("credential with id='%s' not found", id)
+			}
+			err = h.credential.Delete(ctx, id)
 			if err != nil {
 				return nil, err
 			}
@@ -180,12 +202,16 @@ func (h credentialHandler) Delete(w http.ResponseWriter, r *http.Request) {
 func (h credentialHandler) GetToken(w http.ResponseWriter, r *http.Request) {
 	cfg := &handlers.HandlerConfig{
 		Action: func() (interface{}, *errors.ServiceError) {
+			projectID := mux.Vars(r)["id"]
 			id := mux.Vars(r)["cred_id"]
 			ctx := r.Context()
 			credential, err := h.credential.Get(ctx, id)
 			if err != nil {
 				return nil, err
 			}
+			if credential.ProjectID != projectID {
+				return nil, errors.NotFound("credential with id='%s' not found", id)
+			}
 
 			return PresentCredentialToken(credential), nil
 		},

components/ambient-api-server/plugins/sessions/dao.go

@@ -17,6 +17,7 @@ type SessionDao interface {
 	FindByIDs(ctx context.Context, ids []string) (SessionList, error)
 	All(ctx context.Context) (SessionList, error)
 	AllByProjectId(ctx context.Context, projectId string) (SessionList, error)
+	ActiveByAgentID(ctx context.Context, agentID string) (*Session, error)
 }
 
 var _ SessionDao = &sqlSessionDao{}
@@ -91,3 +92,15 @@ func (d *sqlSessionDao) AllByProjectId(ctx context.Context, projectId string) (S
 	}
 	return sessions, nil
 }
+
+func (d *sqlSessionDao) ActiveByAgentID(ctx context.Context, agentID string) (*Session, error) {
+	g2 := (*d.sessionFactory).New(ctx)
+	var session Session
+	err := g2.Where("agent_id = ? AND phase IN (?)", agentID, []string{"Pending", "Creating", "Running"}).
+		Order("created_at DESC").
+		Take(&session).Error
+	if err != nil {
+		return nil, err
+	}
+	return &session, nil
+}

components/ambient-api-server/plugins/sessions/mock_dao.go

@@ -77,3 +77,13 @@ func (d *sessionDaoMock) AllByProjectId(ctx context.Context, projectId string) (
 	}
 	return filtered, nil
 }
+
+func (d *sessionDaoMock) ActiveByAgentID(ctx context.Context, agentID string) (*Session, error) {
+	activePhases := map[string]bool{"Pending": true, "Creating": true, "Running": true}
+	for _, s := range d.sessions {
+		if s.AgentId != nil && *s.AgentId == agentID && s.Phase != nil && activePhases[*s.Phase] {
+			return s, nil
+		}
+	}
+	return nil, gorm.ErrRecordNotFound
+}

components/ambient-api-server/plugins/sessions/service.go

@@ -22,6 +22,7 @@ type SessionService interface {
 	UpdateStatus(ctx context.Context, id string, patch *SessionStatusPatchRequest) (*Session, *errors.ServiceError)
 	Start(ctx context.Context, id string) (*Session, *errors.ServiceError)
 	Stop(ctx context.Context, id string) (*Session, *errors.ServiceError)
+	ActiveByAgentID(ctx context.Context, agentID string) (*Session, *errors.ServiceError)
 
 	FindByIDs(ctx context.Context, ids []string) (SessionList, *errors.ServiceError)
 
@@ -265,6 +266,14 @@ func (s *sqlSessionService) Start(ctx context.Context, id string) (*Session, *er
 	return session, nil
 }
 
+func (s *sqlSessionService) ActiveByAgentID(ctx context.Context, agentID string) (*Session, *errors.ServiceError) {
+	session, err := s.sessionDao.ActiveByAgentID(ctx, agentID)
+	if err != nil {
+		return nil, nil
+	}
+	return session, nil
+}
+
 func (s *sqlSessionService) Stop(ctx context.Context, id string) (*Session, *errors.ServiceError) {
 	session, err := s.sessionDao.Get(ctx, id)
 	if err != nil {

components/ambient-control-plane/cmd/ambient-control-plane/main.go

@@ -148,6 +148,9 @@ func runKubeMode(ctx context.Context, cfg *config.ControlPlaneConfig) error {
 		CPRuntimeNamespace:    cfg.CPRuntimeNamespace,
 		CPTokenURL:            cfg.CPTokenURL,
 		CPTokenPublicKey:      string(kp.PublicKeyPEM),
+		HTTPProxy:             cfg.HTTPProxy,
+		HTTPSProxy:            cfg.HTTPSProxy,
+		NoProxy:               cfg.NoProxy,
 	}
 
 	conn, err := grpc.NewClient(cfg.GRPCServerAddr, grpc.WithTransportCredentials(grpcCredentials(cfg.GRPCUseTLS)))

components/ambient-control-plane/internal/config/config.go

@@ -39,6 +39,9 @@ type ControlPlaneConfig struct {
 	ProjectKubeTokenFile string
 	CPTokenListenAddr    string
 	CPTokenURL           string
+	HTTPProxy            string
+	HTTPSProxy           string
+	NoProxy              string
 }
 
 func Load() (*ControlPlaneConfig, error) {
@@ -75,6 +78,9 @@ func Load() (*ControlPlaneConfig, error) {
 		ProjectKubeTokenFile: os.Getenv("PROJECT_KUBE_TOKEN_FILE"),
 		CPTokenListenAddr:    envOrDefault("CP_TOKEN_LISTEN_ADDR", ":8080"),
 		CPTokenURL:           os.Getenv("CP_TOKEN_URL"),
+		HTTPProxy:            os.Getenv("HTTP_PROXY"),
+		HTTPSProxy:           os.Getenv("HTTPS_PROXY"),
+		NoProxy:              os.Getenv("NO_PROXY"),
 	}
 
 	if cfg.MCPAPIServerURL == "" {

components/ambient-control-plane/internal/kubeclient/kubeclient.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/rs/zerolog"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -201,7 +202,7 @@ func (kc *KubeClient) DeletePod(ctx context.Context, namespace, name string, opt
 }
 
 func (kc *KubeClient) DeletePodsByLabel(ctx context.Context, namespace, labelSelector string) error {
-	return kc.dynamic.Resource(PodGVR).Namespace(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{LabelSelector: labelSelector})
+	return kc.deleteCollectionWithFallback(ctx, PodGVR, namespace, labelSelector)
 }
 
 // Service operations
@@ -214,7 +215,7 @@ func (kc *KubeClient) CreateService(ctx context.Context, obj *unstructured.Unstr
 }
 
 func (kc *KubeClient) DeleteServicesByLabel(ctx context.Context, namespace, labelSelector string) error {
-	return kc.dynamic.Resource(ServiceGVR).Namespace(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{LabelSelector: labelSelector})
+	return kc.deleteCollectionWithFallback(ctx, ServiceGVR, namespace, labelSelector)
 }
 
 // Secret operations
@@ -231,7 +232,7 @@ func (kc *KubeClient) UpdateSecret(ctx context.Context, obj *unstructured.Unstru
 }
 
 func (kc *KubeClient) DeleteSecretsByLabel(ctx context.Context, namespace, labelSelector string) error {
-	return kc.dynamic.Resource(SecretGVR).Namespace(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{LabelSelector: labelSelector})
+	return kc.deleteCollectionWithFallback(ctx, SecretGVR, namespace, labelSelector)
 }
 
 // ServiceAccount operations
@@ -244,7 +245,7 @@ func (kc *KubeClient) CreateServiceAccount(ctx context.Context, obj *unstructure
 }
 
 func (kc *KubeClient) DeleteServiceAccountsByLabel(ctx context.Context, namespace, labelSelector string) error {
-	return kc.dynamic.Resource(ServiceAccountGVR).Namespace(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{LabelSelector: labelSelector})
+	return kc.deleteCollectionWithFallback(ctx, ServiceAccountGVR, namespace, labelSelector)
 }
 
 // Role operations
@@ -257,11 +258,38 @@ func (kc *KubeClient) CreateRole(ctx context.Context, obj *unstructured.Unstruct
 }
 
 func (kc *KubeClient) DeleteRolesByLabel(ctx context.Context, namespace, labelSelector string) error {
-	return kc.dynamic.Resource(RoleGVR).Namespace(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{LabelSelector: labelSelector})
+	return kc.deleteCollectionWithFallback(ctx, RoleGVR, namespace, labelSelector)
 }
 
 func (kc *KubeClient) DeleteRoleBindingsByLabel(ctx context.Context, namespace, labelSelector string) error {
-	return kc.dynamic.Resource(RoleBindingGVR).Namespace(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{LabelSelector: labelSelector})
+	return kc.deleteCollectionWithFallback(ctx, RoleBindingGVR, namespace, labelSelector)
+}
+
+func (kc *KubeClient) deleteCollectionWithFallback(ctx context.Context, gvr schema.GroupVersionResource, namespace, labelSelector string) error {
+	err := kc.dynamic.Resource(gvr).Namespace(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{LabelSelector: labelSelector})
+	if err == nil {
+		return nil
+	}
+	if !k8serrors.IsForbidden(err) {
+		return err
+	}
+
+	kc.logger.Warn().Str("resource", gvr.Resource).Str("namespace", namespace).Msg("deletecollection forbidden, falling back to list+delete")
+
+	list, listErr := kc.dynamic.Resource(gvr).Namespace(namespace).List(ctx, metav1.ListOptions{LabelSelector: labelSelector})
+	if listErr != nil {
+		return fmt.Errorf("fallback list %s: %w", gvr.Resource, listErr)
+	}
+
+	var lastErr error
+	for i := range list.Items {
+		name := list.Items[i].GetName()
+		if delErr := kc.dynamic.Resource(gvr).Namespace(namespace).Delete(ctx, name, metav1.DeleteOptions{}); delErr != nil && !k8serrors.IsNotFound(delErr) {
+			kc.logger.Warn().Err(delErr).Str("resource", gvr.Resource).Str("name", name).Msg("fallback delete failed")
+			lastErr = delErr
+		}
+	}
+	return lastErr
 }
 
 func (kc *KubeClient) GetNetworkPolicy(ctx context.Context, namespace, name string) (*unstructured.Unstructured, error) {

components/ambient-control-plane/internal/reconciler/kube_reconciler.go

@@ -40,6 +40,9 @@ type KubeReconcilerConfig struct {
 	CPRuntimeNamespace    string
 	CPTokenURL            string
 	CPTokenPublicKey      string
+	HTTPProxy             string
+	HTTPSProxy            string
+	NoProxy               string
 }
 
 type SimpleKubeReconciler struct {
@@ -441,8 +444,12 @@ func (r *SimpleKubeReconciler) ensurePod(ctx context.Context, namespace string,
 	}
 
 	if useMCPSidecar {
-		containers = append(containers, r.buildMCPSidecar(session.ID))
-		r.logger.Info().Str("session_id", session.ID).Msg("MCP sidecar enabled for session")
+		if r.cfg.CPTokenURL == "" || r.cfg.CPTokenPublicKey == "" {
+			r.logger.Warn().Str("session_id", session.ID).Msg("MCP sidecar skipped: CP_TOKEN_URL or CPTokenPublicKey not configured")
+		} else {
+			containers = append(containers, r.buildMCPSidecar(session.ID))
+			r.logger.Info().Str("session_id", session.ID).Msg("MCP sidecar enabled for session")
+		}
 	}
 
 	pod := &unstructured.Unstructured{
@@ -637,6 +644,16 @@ func (r *SimpleKubeReconciler) buildEnv(ctx context.Context, session types.Sessi
 		}
 	}
 
+	if r.cfg.HTTPProxy != "" {
+		env = append(env, envVar("HTTP_PROXY", r.cfg.HTTPProxy))
+	}
+	if r.cfg.HTTPSProxy != "" {
+		env = append(env, envVar("HTTPS_PROXY", r.cfg.HTTPSProxy))
+	}
+	if r.cfg.NoProxy != "" {
+		env = append(env, envVar("NO_PROXY", r.cfg.NoProxy))
+	}
+
 	return env
 }
 
@@ -825,6 +842,15 @@ func (r *SimpleKubeReconciler) buildMCPSidecar(sessionID string) interface{} {
 		envVar("AMBIENT_CP_TOKEN_PUBLIC_KEY", r.cfg.CPTokenPublicKey),
 		envVar("SESSION_ID", sessionID),
 	}
+	if r.cfg.HTTPProxy != "" {
+		env = append(env, envVar("HTTP_PROXY", r.cfg.HTTPProxy))
+	}
+	if r.cfg.HTTPSProxy != "" {
+		env = append(env, envVar("HTTPS_PROXY", r.cfg.HTTPSProxy))
+	}
+	if r.cfg.NoProxy != "" {
+		env = append(env, envVar("NO_PROXY", r.cfg.NoProxy))
+	}
 	return map[string]interface{}{
 		"name":            "ambient-mcp",
 		"image":           mcpImage,