fix(api): restrict survey triage to reviewer groups by confidentiality

api/confidentiality_idor_test.go

@@ -204,49 +204,37 @@ func TestSurveyResponseMetadata_ACLGated(t *testing.T) {
 
 	t.Run("triage metadata GET denies external as 404", func(t *testing.T) {
 		c, w := CreateTestGinContext("GET", fmt.Sprintf("/triage/survey_responses/%s/metadata", responseID))
-		TestUsers.External.SetContext(c)
+		asTriageUser(t, c) // not a reviewer
 
 		server.GetTriageSurveyResponseMetadata(c, responseID)
 
 		assert.Equal(t, http.StatusNotFound, w.Code)
 	})
 }
 
-func TestListTriageSurveyResponses_FiltersByACL(t *testing.T) {
-	// Previously this endpoint had a TODO for the ACL filter and returned
-	// every survey response in the system regardless of caller identity.
+func TestListTriageSurveyResponses_NonReviewerSeesNothing(t *testing.T) {
+	// A caller in neither reviewer group must see an empty list with total=0
+	// even when the store holds rows they own (#357, #1100).
 
 	gin.SetMode(gin.TestMode)
 
 	respStore := newMockSurveyResponseStore()
 	saveSurveyStores(t, nil, respStore)
 
 	surveyID := uuid.New()
-
-	// Three responses: owner-A, owner-B, owner-A confidential.
 	idA := seedSurveyResponse(respStore, surveyID, ResponseStatusReadyForReview, TestUsers.Owner.InternalUUID)
-	idB := seedSurveyResponse(respStore, surveyID, ResponseStatusReadyForReview, TestUsers.Writer.InternalUUID)
 	idC := seedSurveyResponse(respStore, surveyID, ResponseStatusReadyForReview, TestUsers.Owner.InternalUUID)
 	conf := true
 	respStore.responses[idC].IsConfidential = &conf
+	respStore.listItems = []SurveyResponseListItem{{Id: &idA}, {Id: &idC}}
+	respStore.listTotal = 2
 
-	// Wire list-items so the mock returns all three from List(...)
-	respStore.listItems = []SurveyResponseListItem{
-		{Id: &idA},
-		{Id: &idB},
-		{Id: &idC},
-	}
-	respStore.listTotal = 3
-
-	// External user has no access to any response — must see empty list,
-	// total=0, even though the underlying store returns 3 rows.
 	server := &Server{}
 	c, w := CreateTestGinContext("GET", "/triage/survey_responses")
-	TestUsers.External.SetContext(c)
+	asTriageUser(t, c)
 
 	server.ListTriageSurveyResponses(c, ListTriageSurveyResponsesParams{})
 
 	assert.Equal(t, http.StatusOK, w.Code)
-	body := w.Body.String()
-	assert.Contains(t, body, `"total":0`, "external user must see total=0 — list count must NOT leak existence of confidential or other-user responses")
+	assert.Contains(t, w.Body.String(), `"total":0`)
 }

api/create_threat_model_from_survey_test.go

@@ -126,9 +126,10 @@ func TestCreateThreatModelFromSurveyResponse_WrongStatus(t *testing.T) {
 	mockResponseStore := &mockSurveyResponseStore{
 		responses: map[uuid.UUID]*SurveyResponse{
 			responseID: {
-				Id:     &responseID,
-				Status: &draftStatus,
-				Owner:  owner,
+				Id:             &responseID,
+				Status:         &draftStatus,
+				Owner:          owner,
+				IsConfidential: new(bool),
 			},
 		},
 		accessMap: map[string]AuthorizationRole{
@@ -142,9 +143,7 @@ func TestCreateThreatModelFromSurveyResponse_WrongStatus(t *testing.T) {
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
 	c.Request = httptest.NewRequest("POST", "/triage/survey_responses/"+responseID.String()+"/create_threat_model", nil)
-	c.Set("userInternalUUID", "user-123")
-	c.Set("userEmail", "alice@example.com")
-	c.Set("userID", "alice-provider-id")
+	asTriageUser(t, c, GroupSecurityReviewers)
 
 	server.CreateThreatModelFromSurveyResponse(c, responseID)
 
@@ -162,7 +161,7 @@ func TestCreateThreatModelFromSurveyResponse_DuplicateTM(t *testing.T) {
 
 	mockResponseStore := &mockSurveyResponseStore{
 		responses: map[uuid.UUID]*SurveyResponse{
-			responseID: {Id: &responseID, Status: &readyStatus, Owner: owner, CreatedThreatModelId: &existingTMID},
+			responseID: {Id: &responseID, Status: &readyStatus, Owner: owner, IsConfidential: new(bool), CreatedThreatModelId: &existingTMID},
 		},
 		accessMap: map[string]AuthorizationRole{responseID.String() + ":user-123": AuthorizationRoleOwner},
 	}
@@ -173,9 +172,7 @@ func TestCreateThreatModelFromSurveyResponse_DuplicateTM(t *testing.T) {
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
 	c.Request = httptest.NewRequest("POST", "/triage/survey_responses/"+responseID.String()+"/create_threat_model", nil)
-	c.Set("userInternalUUID", "user-123")
-	c.Set("userEmail", "alice@example.com")
-	c.Set("userID", "alice")
+	asTriageUser(t, c, GroupSecurityReviewers)
 
 	server.CreateThreatModelFromSurveyResponse(c, responseID)
 	assert.Equal(t, http.StatusConflict, w.Code)
@@ -190,7 +187,7 @@ func TestCreateThreatModelFromSurveyResponse_NilOwner(t *testing.T) {
 
 	mockResponseStore := &mockSurveyResponseStore{
 		responses: map[uuid.UUID]*SurveyResponse{
-			responseID: {Id: &responseID, Status: &readyStatus, Owner: nil},
+			responseID: {Id: &responseID, Status: &readyStatus, Owner: nil, IsConfidential: new(bool)},
 		},
 		accessMap: map[string]AuthorizationRole{responseID.String() + ":user-123": AuthorizationRoleOwner},
 	}
@@ -201,9 +198,7 @@ func TestCreateThreatModelFromSurveyResponse_NilOwner(t *testing.T) {
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
 	c.Request = httptest.NewRequest("POST", "/triage/survey_responses/"+responseID.String()+"/create_threat_model", nil)
-	c.Set("userInternalUUID", "user-123")
-	c.Set("userEmail", "alice@example.com")
-	c.Set("userID", "alice")
+	asTriageUser(t, c, GroupSecurityReviewers)
 
 	server.CreateThreatModelFromSurveyResponse(c, responseID)
 	assert.Equal(t, http.StatusInternalServerError, w.Code)
@@ -230,9 +225,7 @@ func TestCreateThreatModelFromSurveyResponse_AccessDenied(t *testing.T) {
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
 	c.Request = httptest.NewRequest("POST", "/triage/survey_responses/"+responseID.String()+"/create_threat_model", nil)
-	c.Set("userInternalUUID", "user-123")
-	c.Set("userEmail", "alice@example.com")
-	c.Set("userID", "alice")
+	asTriageUser(t, c)
 
 	server.CreateThreatModelFromSurveyResponse(c, responseID)
 	// T5 (#357): access denied collapses to 404 to avoid existence disclosure.

api/server_survey.go

@@ -141,18 +141,18 @@ func (s *Server) UpdateIntakeSurveyResponseMetadataByKey(c *gin.Context, surveyR
 // Survey Response Metadata Methods - Triage (read-only)
 
 // GetTriageSurveyResponseMetadata gets triage survey response metadata
-// SEM@368e91d91cb110162c64b6ea10d49562a59bf3f0: authorize reader access then list all triage survey response metadata entries (reads DB)
+// SEM@49259ca96d8bf22bf9c7a384c528d9a39bebe16d: authorize triage reviewer access then list triage survey response metadata entries (reads DB)
 func (s *Server) GetTriageSurveyResponseMetadata(c *gin.Context, surveyResponseId SurveyResponseId) {
-	if _, ok := RequireSurveyResponseAccess(c, surveyResponseId, AuthorizationRoleReader); !ok {
+	if _, ok := RequireTriageAccess(c, surveyResponseId); !ok {
 		return
 	}
 	s.surveyResponseMetadata.List(c)
 }
 
 // GetTriageSurveyResponseMetadataByKey gets triage survey response metadata by key
-// SEM@368e91d91cb110162c64b6ea10d49562a59bf3f0: authorize reader access then fetch triage survey response metadata by key (reads DB)
+// SEM@49259ca96d8bf22bf9c7a384c528d9a39bebe16d: authorize triage reviewer access then fetch triage survey response metadata by key (reads DB)
 func (s *Server) GetTriageSurveyResponseMetadataByKey(c *gin.Context, surveyResponseId SurveyResponseId, key MetadataKey) {
-	if _, ok := RequireSurveyResponseAccess(c, surveyResponseId, AuthorizationRoleReader); !ok {
+	if _, ok := RequireTriageAccess(c, surveyResponseId); !ok {
 		return
 	}
 	s.surveyResponseMetadata.GetByKey(c)
@@ -161,22 +161,31 @@ func (s *Server) GetTriageSurveyResponseMetadataByKey(c *gin.Context, surveyResp
 // Survey Response Triage Notes Methods - Triage (create + read)
 
 // ListTriageSurveyResponseTriageNotes lists triage notes for a survey response
-// SEM@28792aa3991e394010e49c040d3db2d5f14a6eff: list triage notes for a survey response via the triage stage (reads DB)
+// SEM@49259ca96d8bf22bf9c7a384c528d9a39bebe16d: authorize triage reviewer access then list triage notes for a survey response (reads DB)
 func (s *Server) ListTriageSurveyResponseTriageNotes(c *gin.Context, surveyResponseId SurveyResponseId, params ListTriageSurveyResponseTriageNotesParams) {
+	if _, ok := RequireTriageAccess(c, surveyResponseId); !ok {
+		return
+	}
 	c.Params = append(c.Params, gin.Param{Key: "survey_response_id", Value: surveyResponseId.String()})
 	s.triageNoteHandler.ListTriageNotes(c)
 }
 
 // CreateTriageSurveyResponseTriageNote creates a triage note
-// SEM@28792aa3991e394010e49c040d3db2d5f14a6eff: store a new triage note for a survey response via the triage stage (mutates shared state)
+// SEM@49259ca96d8bf22bf9c7a384c528d9a39bebe16d: authorize triage reviewer access then store a new triage note (mutates shared state)
 func (s *Server) CreateTriageSurveyResponseTriageNote(c *gin.Context, surveyResponseId SurveyResponseId) {
+	if _, ok := RequireTriageAccess(c, surveyResponseId); !ok {
+		return
+	}
 	c.Params = append(c.Params, gin.Param{Key: "survey_response_id", Value: surveyResponseId.String()})
 	s.triageNoteHandler.CreateTriageNote(c)
 }
 
 // GetTriageSurveyResponseTriageNote gets a specific triage note
-// SEM@28792aa3991e394010e49c040d3db2d5f14a6eff: fetch a single triage note for a survey response via the triage stage (reads DB)
+// SEM@49259ca96d8bf22bf9c7a384c528d9a39bebe16d: authorize triage reviewer access then fetch a single triage note (reads DB)
 func (s *Server) GetTriageSurveyResponseTriageNote(c *gin.Context, surveyResponseId SurveyResponseId, triageNoteId TriageNoteId) {
+	if _, ok := RequireTriageAccess(c, surveyResponseId); !ok {
+		return
+	}
 	c.Params = append(c.Params, gin.Param{Key: "survey_response_id", Value: surveyResponseId.String()})
 	c.Params = append(c.Params, gin.Param{Key: "triage_note_id", Value: strconv.Itoa(triageNoteId)})
 	s.triageNoteHandler.GetTriageNote(c)

api/survey_handlers.go

@@ -110,7 +110,7 @@ func (s *Server) ListAdminSurveys(c *gin.Context, params ListAdminSurveysParams)
 
 // CreateAdminSurvey creates a new survey.
 // POST /admin/surveys
-// SEM@ed719f753d3a02b551f5a7a01daff2f01a221118: store a new survey template and emit a webhook event (mutates shared state)
+// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: store a new survey template and emit a webhook event (mutates shared state)
 func (s *Server) CreateAdminSurvey(c *gin.Context) {
 	logger := slogging.Get()
 	ctx := c.Request.Context()
@@ -227,7 +227,7 @@ func (s *Server) GetAdminSurvey(c *gin.Context, surveyId SurveyId) {
 
 // UpdateAdminSurvey fully updates a survey.
 // PUT /admin/surveys/{survey_id}
-// SEM@ed719f753d3a02b551f5a7a01daff2f01a221118: fully replace a non-archived survey and emit a webhook event (mutates shared state)
+// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: fully replace a non-archived survey and emit a webhook event (mutates shared state)
 func (s *Server) UpdateAdminSurvey(c *gin.Context, surveyId SurveyId) {
 	logger := slogging.Get()
 	ctx := c.Request.Context()
@@ -856,10 +856,23 @@ func (s *Server) UpdateIntakeSurveyResponse(c *gin.Context, surveyResponseId Sur
 	c.JSON(http.StatusOK, updated)
 }
 
+// validateIntakeStatusTarget rejects any status an intake caller may not set.
+// The submitter may only move a response to draft or submitted; approval and
+// review states belong to triage reviewers (#1100). PUT and create carry no
+// status field, so PATCH is the only intake path that reaches this check.
+// SEM@fea6b3be63afa09c0668f3eaa2f51f933c5e05f1: validate that an intake caller may set a survey response status only to draft or submitted (pure)
+func validateIntakeStatusTarget(status string) error {
+	if status != ResponseStatusDraft && status != ResponseStatusSubmitted {
+		return fmt.Errorf("status %q cannot be set through intake endpoints; only %q or %q is allowed",
+			status, ResponseStatusDraft, ResponseStatusSubmitted)
+	}
+	return nil
+}
+
 // PatchIntakeSurveyResponse partially updates a survey response.
 // PATCH /intake/survey_responses/{response_id}
 // Supports status transitions: draft->submitted, needs_revision->submitted
-// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: apply a JSON Patch to a draft or needs-revision survey response and handle status transitions (mutates DB)
+// SEM@fea6b3be63afa09c0668f3eaa2f51f933c5e05f1: apply a JSON Patch to a draft or needs-revision survey response, allowing only draft or submitted status targets (mutates DB)
 func (s *Server) PatchIntakeSurveyResponse(c *gin.Context, surveyResponseId SurveyResponseId, _ PatchIntakeSurveyResponseParams) {
 	logger := slogging.Get()
 	ctx := c.Request.Context()
@@ -924,6 +937,10 @@ func (s *Server) PatchIntakeSurveyResponse(c *gin.Context, surveyResponseId Surv
 	// transition never lets the CAS-guarded write commit only to fail on an
 	// unrelated validation afterward.
 	if hasStatusChange && patched.Status != nil && *patched.Status != *existing.Status {
+		if err := validateIntakeStatusTarget(*patched.Status); err != nil {
+			HandleRequestError(c, InvalidInputError(err.Error()))
+			return
+		}
 		if err := ValidateSurveyResponseStatusTransition(*patched.Status, nil); err != nil {
 			HandleRequestError(c, InvalidInputError(err.Error())) // validation failure, not a conflict (#987)
 			return
@@ -1054,13 +1071,12 @@ func (s *Server) DeleteIntakeSurveyResponse(c *gin.Context, surveyResponseId Sur
 
 // ListTriageSurveyResponses returns survey responses for triage.
 // GET /triage/survey_responses
-// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: list survey responses visible to the triage reviewer, filtered by per-row ACL (reads DB)
+// SEM@49259ca96d8bf22bf9c7a384c528d9a39bebe16d: list survey responses scoped to the caller's reviewer groups by confidentiality (reads DB)
 func (s *Server) ListTriageSurveyResponses(c *gin.Context, params ListTriageSurveyResponsesParams) {
 	logger := slogging.Get()
 	ctx := c.Request.Context()
 
-	userInternalUUID, ok := getUserUUID(c)
-	if !ok {
+	if _, ok := getUserUUID(c); !ok {
 		return
 	}
 
@@ -1083,54 +1099,45 @@ func (s *Server) ListTriageSurveyResponses(c *gin.Context, params ListTriageSurv
 		}
 	}
 
-	// Authorization model (T5, #357):
-	// The triage queue is intentionally not owner-scoped — security reviewers
-	// must see responses they did not submit. Per-row ACL (seeded at create)
-	// already encodes who can see what:
-	//   - non-confidential responses grant Security Reviewers group owner role
-	//   - confidential responses grant Confidential Project Reviewers group owner role
-	//   - the submitter has direct owner role
-	// We unconditionally fetch the page from the store (no ACL pre-filter) and
-	// apply HasAccess in-memory to the returned items + recompute total. This
-	// keeps the list, per-row, and confidentiality decisions consistent and
-	// closes the previous TODO that left this endpoint enumerating all
-	// responses regardless of caller identity.
-	//
-	// We over-fetch here (full table for the requested filter) and paginate in
-	// memory; this is an N-roundtrip pattern that mirrors ListWithCounts on
-	// threat models and is acceptable for the triage queue size today. If the
-	// queue grows past a few thousand rows this should move to a SQL JOIN
-	// against survey_response_access + group_members (tracked separately).
-	const overFetchLimit = 10000
-	allItems, _, err := GlobalSurveyResponseStore.List(ctx, overFetchLimit, 0, filters)
+	// Authorization model (#1100): only reviewer-group members may triage.
+	// Security Reviewers see non-confidential responses, Confidential Project
+	// Reviewers see confidential ones, both see all; ownership and the row ACL
+	// grant nothing here. The confidentiality scope is pushed into the store
+	// query so pagination and totals stay correct.
+	confidential, hasScope, err := triageReviewerScope(c)
 	if err != nil {
-		logger.Error("Failed to list survey responses: %v", err)
-		c.JSON(http.StatusInternalServerError, Error{
-			Error:            ErrorError(errcode.ServerError),
-			ErrorDescription: "Failed to list survey responses",
-		})
+		logger.Error("Failed to resolve triage reviewer scope: %v", err)
+		HandleRequestError(c, ServerError("Failed to check access"))
 		return
 	}
-
-	visible := FilterSurveyResponseListItemsByAccess(ctx, userInternalUUID, allItems, AuthorizationRoleReader)
-	total := len(visible)
-
-	if offset >= total {
+	if !hasScope {
 		c.JSON(http.StatusOK, ListSurveyResponsesResponse{
 			SurveyResponses: []SurveyResponseListItem{},
-			Total:           total,
+			Total:           0,
 			Limit:           limit,
 			Offset:          offset,
 		})
 		return
 	}
-	end := offset + limit
-	if end > total || limit <= 0 {
-		end = total
+	if confidential != nil {
+		if filters == nil {
+			filters = &SurveyResponseFilters{}
+		}
+		filters.IsConfidential = confidential
+	}
+
+	items, total, err := GlobalSurveyResponseStore.List(ctx, limit, offset, filters)
+	if err != nil {
+		logger.Error("Failed to list survey responses: %v", err)
+		c.JSON(http.StatusInternalServerError, Error{
+			Error:            ErrorError(errcode.ServerError),
+			ErrorDescription: "Failed to list survey responses",
+		})
+		return
 	}
 
 	c.JSON(http.StatusOK, ListSurveyResponsesResponse{
-		SurveyResponses: visible[offset:end],
+		SurveyResponses: items,
 		Total:           total,
 		Limit:           limit,
 		Offset:          offset,
@@ -1139,9 +1146,9 @@ func (s *Server) ListTriageSurveyResponses(c *gin.Context, params ListTriageSurv
 
 // GetTriageSurveyResponse returns a specific survey response for triage.
 // GET /triage/survey_responses/{response_id}
-// SEM@368e91d91cb110162c64b6ea10d49562a59bf3f0: fetch an authorized survey response for triage review (reads DB)
+// SEM@49259ca96d8bf22bf9c7a384c528d9a39bebe16d: fetch a survey response for a caller in the matching triage reviewer group (reads DB)
 func (s *Server) GetTriageSurveyResponse(c *gin.Context, surveyResponseId SurveyResponseId) {
-	response, ok := RequireSurveyResponseAccess(c, surveyResponseId, AuthorizationRoleReader)
+	response, ok := RequireTriageAccess(c, surveyResponseId)
 	if !ok {
 		return
 	}
@@ -1151,12 +1158,12 @@ func (s *Server) GetTriageSurveyResponse(c *gin.Context, surveyResponseId Survey
 // PatchTriageSurveyResponse partially updates a survey response for triage.
 // PATCH /triage/survey_responses/{response_id}
 // Supports status transitions: submitted->ready_for_review, submitted->needs_revision, ready_for_review->needs_revision
-// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: apply a JSON Patch to update a survey response's triage status (mutates DB)
+// SEM@49259ca96d8bf22bf9c7a384c528d9a39bebe16d: apply a JSON Patch to a survey response's triage status for a matching triage reviewer (mutates DB)
 func (s *Server) PatchTriageSurveyResponse(c *gin.Context, surveyResponseId SurveyResponseId) {
 	logger := slogging.Get()
 	ctx := c.Request.Context()
 
-	existing, ok := RequireSurveyResponseAccess(c, surveyResponseId, AuthorizationRoleOwner)
+	existing, ok := RequireTriageAccess(c, surveyResponseId)
 	if !ok {
 		return
 	}
@@ -1498,18 +1505,18 @@ func createThreatModelFromResponse(ctx context.Context, response *SurveyResponse
 
 // CreateThreatModelFromSurveyResponse creates a threat model from an approved survey response.
 // POST /triage/survey_responses/{response_id}/create_threat_model
-// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: build a threat model from a ready-for-review survey response and notify webhooks (mutates DB)
+// SEM@49259ca96d8bf22bf9c7a384c528d9a39bebe16d: build a threat model from a ready-for-review response for a matching triage reviewer and notify webhooks (mutates DB)
 func (s *Server) CreateThreatModelFromSurveyResponse(c *gin.Context, surveyResponseId SurveyResponseId) {
 	logger := slogging.Get()
 	ctx := c.Request.Context()
 
-	// Step 1: Load + access check via centralized helper
-	response, ok := RequireSurveyResponseAccess(c, surveyResponseId, AuthorizationRoleOwner)
+	// Step 1: Load + triage reviewer check via centralized helper
+	response, ok := RequireTriageAccess(c, surveyResponseId)
 	if !ok {
 		return
 	}
 
-	// Step 2: Extract user identity (already validated by RequireSurveyResponseAccess)
+	// Step 2: Extract user identity (already validated by RequireTriageAccess)
 	user, err := GetAuthenticatedUser(c)
 	if err != nil {
 		HandleRequestError(c, err)