Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/csm-portal/backend/cmd/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ func main() {
mux.HandleFunc("GET /projects/{id}", projectHandler.GetProject)
mux.HandleFunc("POST /projects/search", projectHandler.SearchProjects)
mux.HandleFunc("POST /projects/{id}/contacts/search", projectHandler.SearchProjectContacts)
mux.HandleFunc("PATCH /projects/{id}", projectHandler.UpdateProject)
mux.HandleFunc("POST /products/search", productHandler.SearchProducts)
mux.HandleFunc("POST /products/{id}/versions/search", productHandler.SearchProductVersions)
mux.HandleFunc("POST /deployments", deploymentHandler.PostDeployment)
Expand Down
6 changes: 6 additions & 0 deletions apps/csm-portal/backend/internal/entity/entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ func (c *Client) SearchProjectContacts(ctx context.Context, projectID string, bo
return c.do(ctx, http.MethodPost, fmt.Sprintf("/projects/%s/contacts/search", url.PathEscape(projectID)), body)
}

// UpdateProject calls PATCH /projects/{id} on the entity service.
// Response is returned as raw JSON; typed response structs are deferred.
func (c *Client) UpdateProject(ctx context.Context, id string, body []byte) ([]byte, error) {
return c.do(ctx, http.MethodPatch, fmt.Sprintf("/projects/%s", url.PathEscape(id)), body)
}

// SearchProducts calls POST /products/search on the entity service.
// Response is returned as raw JSON; field filtering to the portal shape is deferred.
func (c *Client) SearchProducts(ctx context.Context, body []byte) ([]byte, error) {
Expand Down
8 changes: 8 additions & 0 deletions apps/csm-portal/backend/internal/handler/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ type mockEntityProjectClient struct {
getProjectFn func(ctx context.Context, id string) ([]byte, error)
searchProjectsFn func(ctx context.Context, body []byte) ([]byte, error)
searchProjectContactsFn func(ctx context.Context, projectID string, body []byte) ([]byte, error)
updateProjectFn func(ctx context.Context, id string, body []byte) ([]byte, error)
}

func (m *mockEntityProjectClient) GetProject(ctx context.Context, id string) ([]byte, error) {
Expand All @@ -336,6 +337,13 @@ func (m *mockEntityProjectClient) SearchProjectContacts(ctx context.Context, pro
return []byte(`{}`), nil
}

func (m *mockEntityProjectClient) UpdateProject(ctx context.Context, id string, body []byte) ([]byte, error) {
if m.updateProjectFn != nil {
return m.updateProjectFn(ctx, id, body)
}
return []byte(`{}`), nil
}

// ----- mock entity product client -----

type mockEntityProductClient struct {
Expand Down
47 changes: 47 additions & 0 deletions apps/csm-portal/backend/internal/handler/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type entityProjectClient interface {
GetProject(ctx context.Context, id string) ([]byte, error)
SearchProjects(ctx context.Context, body []byte) ([]byte, error)
SearchProjectContacts(ctx context.Context, projectID string, body []byte) ([]byte, error)
UpdateProject(ctx context.Context, id string, body []byte) ([]byte, error)
}

// ProjectHandler handles HTTP requests for project operations, delegating to the
Expand Down Expand Up @@ -146,3 +147,49 @@ func (h *ProjectHandler) SearchProjectContacts(w http.ResponseWriter, r *http.Re

writeJSON(w, http.StatusOK, result)
}

// UpdateProject handles PATCH /projects/{id}.
// The endpoint is path-scoped, so the request body is capped and forwarded to the
// entity service as-is (no fields are injected) and the response is returned verbatim.
// The entity service is the source of truth for field-level validation (e.g. at
// least one field must be provided); the backend has no role-based access control
// layer yet, so any authenticated user may invoke this today, matching the
// existing convention on other PATCH endpoints in this codebase.
func (h *ProjectHandler) UpdateProject(w http.ResponseWriter, r *http.Request) {
user := middleware.UserInfoFromContext(r.Context())
if user == nil {
writeError(w, http.StatusUnauthorized, ErrMsgUnauthorized)
return
}

id := r.PathValue("id")
if id == "" || !uuidRe.MatchString(id) {
writeError(w, http.StatusBadRequest, ErrMsgInvalidUUID)
return
}

r.Body = http.MaxBytesReader(w, r.Body, maxRequestBodyBytes)
body, err := io.ReadAll(r.Body)
if err != nil {
if _, ok := err.(*http.MaxBytesError); ok {
writeError(w, http.StatusRequestEntityTooLarge, ErrMsgTooLarge)
return
}
writeError(w, http.StatusBadRequest, errMsgReadBody)
return
}

if !json.Valid(body) {
writeError(w, http.StatusBadRequest, ErrMsgBadRequest)
return
}

result, err := h.entity.UpdateProject(r.Context(), id, body)
if err != nil {
slog.ErrorContext(r.Context(), "entity UpdateProject failed", "userID", user.UserID, "projectID", id, "err", err)
mapUpstreamError(w, err, "Failed to update project.")
return
}

writeJSON(w, http.StatusOK, result)
}
112 changes: 112 additions & 0 deletions apps/csm-portal/backend/internal/handler/projects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,3 +281,115 @@ func TestSearchProjectContacts(t *testing.T) {
}
})
}

func TestUpdateProject(t *testing.T) {
const projectID = "11111111-1111-1111-1111-111111111111"

t.Run("requires authenticated user", func(t *testing.T) {
h := NewProjectHandler(&mockEntityProjectClient{})
r := httptest.NewRequest(http.MethodPatch, "/projects/"+projectID, strings.NewReader(`{"hasAgent":true}`))
r.SetPathValue("id", projectID)
w := httptest.NewRecorder()
h.UpdateProject(w, r)
assertStatus(t, w, http.StatusUnauthorized)
assertErrorMessage(t, w, ErrMsgUnauthorized)
assertContentType(t, w, "application/json")
})

t.Run("rejects empty project ID", func(t *testing.T) {
h := NewProjectHandler(&mockEntityProjectClient{})
r := withUser(httptest.NewRequest(http.MethodPatch, "/projects/", strings.NewReader(`{"hasAgent":true}`)))
w := httptest.NewRecorder()
h.UpdateProject(w, r)
assertStatus(t, w, http.StatusBadRequest)
assertErrorMessage(t, w, ErrMsgInvalidUUID)
assertContentType(t, w, "application/json")
})

t.Run("rejects non-UUID project ID", func(t *testing.T) {
h := NewProjectHandler(&mockEntityProjectClient{})
r := withUser(httptest.NewRequest(http.MethodPatch, "/projects/proj-42", strings.NewReader(`{"hasAgent":true}`)))
r.SetPathValue("id", "proj-42")
w := httptest.NewRecorder()
h.UpdateProject(w, r)
assertStatus(t, w, http.StatusBadRequest)
assertErrorMessage(t, w, ErrMsgInvalidUUID)
assertContentType(t, w, "application/json")
})

t.Run("rejects body exceeding 1 MiB", func(t *testing.T) {
h := NewProjectHandler(&mockEntityProjectClient{})
r := withUser(httptest.NewRequest(http.MethodPatch, "/projects/"+projectID, strings.NewReader(strings.Repeat("x", maxRequestBodyBytes+1))))
r.SetPathValue("id", projectID)
w := httptest.NewRecorder()
h.UpdateProject(w, r)
assertStatus(t, w, http.StatusRequestEntityTooLarge)
assertErrorMessage(t, w, ErrMsgTooLarge)
assertContentType(t, w, "application/json")
})

t.Run("rejects invalid JSON body", func(t *testing.T) {
h := NewProjectHandler(&mockEntityProjectClient{})
r := withUser(httptest.NewRequest(http.MethodPatch, "/projects/"+projectID, strings.NewReader(`not-json`)))
r.SetPathValue("id", projectID)
w := httptest.NewRecorder()
h.UpdateProject(w, r)
assertStatus(t, w, http.StatusBadRequest)
assertErrorMessage(t, w, ErrMsgBadRequest)
assertContentType(t, w, "application/json")
})

t.Run("forwards body verbatim and returns upstream response", func(t *testing.T) {
var capturedID string
var capturedBody []byte
reqBody := `{"endDateClosureState":"in_progress","complianceViolationClosureState":"completed"}`
client := &mockEntityProjectClient{
updateProjectFn: func(_ context.Context, id string, body []byte) ([]byte, error) {
capturedID = id
capturedBody = body
return []byte(`{"message":"Project updated.","project":{"id":"` + projectID + `","updatedBy":"user@example.com","updatedOn":"2026-01-01T00:00:00Z","closureState":"in_progress","endDateClosureState":"in_progress","invoiceDueDateClosureState":null,"complianceViolationClosureState":"completed"}}`), nil
},
}
h := NewProjectHandler(client)
r := withUser(httptest.NewRequest(http.MethodPatch, "/projects/"+projectID, strings.NewReader(reqBody)))
r.SetPathValue("id", projectID)
w := httptest.NewRecorder()
h.UpdateProject(w, r)

assertStatus(t, w, http.StatusOK)
assertContentType(t, w, "application/json")

if capturedID != projectID {
t.Errorf("projectID = %q, want %q", capturedID, projectID)
}
if string(capturedBody) != reqBody {
t.Errorf("upstream body = %q, want verbatim %q", string(capturedBody), reqBody)
}

resp := decodeJSON[map[string]any](t, w)
if resp["message"] != "Project updated." {
t.Errorf("message = %v, want %q", resp["message"], "Project updated.")
}
})

t.Run("upstream errors are mapped correctly", func(t *testing.T) {
for _, tc := range upstreamErrors("Failed to update project.") {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
client := &mockEntityProjectClient{
updateProjectFn: func(_ context.Context, _ string, _ []byte) ([]byte, error) {
return nil, tc.err
},
}
h := NewProjectHandler(client)
r := withUser(httptest.NewRequest(http.MethodPatch, "/projects/"+projectID, strings.NewReader(`{"hasAgent":true}`)))
r.SetPathValue("id", projectID)
w := httptest.NewRecorder()
h.UpdateProject(w, r)
assertStatus(t, w, tc.wantCode)
assertErrorMessage(t, w, tc.wantMsg)
assertContentType(t, w, "application/json")
})
}
})
}
117 changes: 117 additions & 0 deletions apps/csm-portal/backend/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,72 @@ paths:
schema:
$ref: '#/components/schemas/ErrorPayload'

patch:
summary: Update a project's closure sub-state fields or KB/agent toggles.
description: >
Request body is forwarded to the integration service as-is and the
response is returned verbatim. At least one field must be provided;
the integration service validates this and rejects an empty update.
Comment on lines +951 to +956

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Close and enforce the project-update payload contract.

The handler forwards {}, null, arrays, scalars, wrongly typed values, and undeclared fields because json.Valid only checks syntax; this contradicts the documented object/minimum-field contract and sends unexpected input upstream.

  • apps/csm-portal/backend/openapi.yaml#L951-L956: state that the BFF rejects malformed, empty, and unsupported update payloads rather than delegating empty-update validation.
  • apps/csm-portal/backend/openapi.yaml#L4999-L5018: add additionalProperties: false.
  • apps/csm-portal/backend/internal/handler/projects.go#L182-L185: validate a non-empty object with only the documented fields and their documented types before forwarding the original bytes.
  • apps/csm-portal/backend/internal/handler/projects_test.go#L331-L340: add rejection cases for {}, null, [], unknown fields, and invalid field types; assert the client is not called.

As per coding guidelines, “Validate and reject unexpected input at the boundary (path params, body size, JSON structure) before forwarding requests to upstream services.”

📍 Affects 3 files
  • apps/csm-portal/backend/openapi.yaml#L951-L956 (this comment)
  • apps/csm-portal/backend/openapi.yaml#L4999-L5018
  • apps/csm-portal/backend/internal/handler/projects.go#L182-L185
  • apps/csm-portal/backend/internal/handler/projects_test.go#L331-L340
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/csm-portal/backend/openapi.yaml` around lines 951 - 956, Close the
project-update payload contract across all affected sites: in
apps/csm-portal/backend/openapi.yaml lines 951-956, document that the BFF
rejects malformed, empty, and unsupported payloads; in
apps/csm-portal/backend/openapi.yaml lines 4999-5018, set additionalProperties
to false; in apps/csm-portal/backend/internal/handler/projects.go lines 182-185,
validate a non-empty object containing only documented fields with their
documented types before forwarding the original bytes; and in
apps/csm-portal/backend/internal/handler/projects_test.go lines 331-340, add
rejection cases for {}, null, [], unknown fields, and invalid types, asserting
the upstream client is not called.

Source: Coding guidelines

operationId: patchProjectsId
parameters:
- name: id
in: path
description: UUID of the project to update.
required: true
schema:
type: string
format: uuid
requestBody:
description: Project update payload.
required: true
content:
application/json:
schema:
$ref: '#/components/schemas/ProjectUpdatePayload'
responses:
"200":
description: Ok
content:
application/json:
schema:
$ref: '#/components/schemas/ProjectUpdateResponse'
"400":
description: BadRequest
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorPayload'
"401":
description: Unauthorized
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorPayload'
"403":
description: Forbidden
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorPayload'
"404":
description: NotFound
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorPayload'
"413":
description: RequestEntityTooLarge
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorPayload'
"500":
description: InternalServerError
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorPayload'

/projects/search:
post:
summary: Search projects.
Expand Down Expand Up @@ -4930,6 +4996,57 @@ components:
type: string
format: date-time

ProjectUpdatePayload:
type: object
minProperties: 1
description: >
At least one field must be provided. hasAgent and hasKbReferences are
simple toggles; the closure-state fields track sub-states of an
Account Closure Process and are only applicable to projects sourced
from the backing data source.
properties:
hasAgent:
type: boolean
hasKbReferences:
type: boolean
endDateClosureState:
type: string
invoiceDueDateClosureState:
type: string
complianceViolationClosureState:
type: string

ProjectUpdateResponse:
type: object
required: [message, project]
properties:
message:
type: string
project:
type: object
required: [id, updatedBy, updatedOn]
properties:
id:
type: string
format: uuid
updatedBy:
type: string
updatedOn:
type: string
format: date-time
closureState:
type: string
nullable: true
endDateClosureState:
type: string
nullable: true
invoiceDueDateClosureState:
type: string
nullable: true
complianceViolationClosureState:
type: string
nullable: true

DeploymentCreatePayload:
type: object
required: [projectId, name, type, description]
Expand Down