Conversation
Adds an opaque-passthrough handler for the ACP closure-state fields
(and existing hasAgent/hasKbReferences toggles) on the Go BFF, mirroring
the entity-service's PATCH /projects/{id} contract. Auth-checks and
UUID-validates the path id, caps and forwards the body verbatim, and
returns the upstream response as-is -- no BFF-level RBAC beyond
"authenticated user", matching this codebase's existing convention for
PATCH endpoints (e.g. PatchCallRequest) of letting the entity service
reject unsupported writes.
📝 WalkthroughWalkthroughAdds a ChangesProject update flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HTTPClient
participant ProjectHandler
participant EntityClient
HTTPClient->>ProjectHandler: PATCH /projects/{id} with JSON body
ProjectHandler->>ProjectHandler: Authenticate and validate request
ProjectHandler->>EntityClient: UpdateProject(ctx, id, body)
EntityClient-->>ProjectHandler: JSON response or error
ProjectHandler-->>HTTPClient: 200 response or mapped error
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)apps/csm-portal/backend/openapi.yamlTraceback (most recent call last): 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@apps/csm-portal/backend/openapi.yaml`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abac8110-401b-44c0-a5fb-405c858156ee
📒 Files selected for processing (6)
apps/csm-portal/backend/cmd/server/main.goapps/csm-portal/backend/internal/entity/entity.goapps/csm-portal/backend/internal/handler/helpers_test.goapps/csm-portal/backend/internal/handler/projects.goapps/csm-portal/backend/internal/handler/projects_test.goapps/csm-portal/backend/openapi.yaml
| 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. |
There was a problem hiding this comment.
🎯 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: addadditionalProperties: 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-L5018apps/csm-portal/backend/internal/handler/projects.go#L182-L185apps/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
|
Closing — this endpoint doesn't belong in this component. The CSM Portal BFF is for the CSM Portal's own end-user sessions (JWT-validated), but the caller that needs to patch project closure-state (the ACP automation job) is M2M with no end user in the loop. That's exactly what |
rksk
changed the title
1 participant
Summary
PATCH /projects/{id}tooperations/csm-integration-service, forwarding to the entity-service's ServiceNow-backed project update operation (merged in [CSM Portal] Add project/account contact search, closure fields, and project update endpoint #1191).csm-integration-service(merged in [CSM Integration Service] Add new Go service for third-party account/project search #1202) exists specifically to serve M2M/third-party consumers through Choreo's API Manager gateway — unlikeapps/csm-portal/backend, which is a BFF for the CSM Portal's own JWT-authenticated end users.apps/csm-portal/backend); that was the wrong component for an M2M caller and has been replaced with this implementation instead.Goals
endDateClosureState,invoiceDueDateClosureState,complianceViolationClosureState) plus the pre-existinghasAgent/hasKbReferencestoggles, through the M2M-facing service, not the BFF.Approach
internal/entity/entity.go: newClient.UpdateProject(PATCH/projects/{id}), matching this file's existing client-method pattern.internal/handler/projects.go: extendedentityProjectClientinterface; newProjectHandler.UpdateProject— path-scoped, UUID-validated, body-capped, JSON-validated, forwarded and returned verbatim.CLAUDE.md); Choreo's API Manager gateway is the trust boundary. A caller with no forwardedx-user-id-tokengets a mapped 401 from the upstream ServiceNow-only operation, which is expected and documented, not worked around.cmd/server/main.go: registered the route.openapi.yaml: documented the operation, including the 401 case per this service's own convention for ServiceNow-backed operations.Test plan
make test(vet + race-detector)make buildgofmt -lon changed files (clean)TestUpdateProject: invalid/empty UUID rejected, body-size cap enforced, invalid JSON rejected, body forwarded verbatim and response returned verbatim, upstream errors mapped correctlyRelease note
PATCH /projects/{id}is now available on the CSM Integration Service for M2M consumers.Documentation
Updated
operations/csm-integration-service/openapi.yaml.Security checks
go vetran cleanRelated PRs
Depends on #1191 (entity-service project update endpoint) and #1202 (csm-integration-service itself).