Skip to content

[Entity-Service] stop leaking internal error tags into user-facing messages - #1259

Closed
rksk wants to merge 1 commit into
wso2-open-operations:mainfrom
rksk:entity-service-strip-internal-error-tags
Closed

rksk wants to merge 1 commit into
wso2-open-operations:mainfrom
rksk:entity-service-strip-internal-error-tags

Conversation

@rksk

@rksk rksk commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Purpose

customer-entity-service sometimes returns a client-facing error message with a literal internal tag prefix, e.g. {"message":"[SERVICENOW_ERROR] State transition rejected"}. The tag is meant to identify which internal/downstream layer raised the error for log correlation, but it was flowing straight through into the field the CSM portal FE displays to CS engineers. Not a security issue (internal-only audience, no secrets/PII in the tag), but it leaks backend/vendor implementation detail and reads as an unhandled bug.

Goals

Strip the leading bracketed internal tag (e.g. [SERVICENOW_ERROR], [ENTITY_SERVICE_ERROR]) from any downstream error message before it is placed into the client-facing error response, while still logging the tag server-side for correlation, and without losing the useful text that follows the tag (e.g. "State transition rejected" still reaches the client).

Approach

Root cause: extractDownstreamMessage in entity-service/internal/servicenow-integration-service/client.go pulls the message field straight out of the downstream error body and uses it verbatim as the Msg on the returned apierror.* type. That same Msg is both logged and written to the HTTP response body in writeServiceError (internal/handler/decode.go), so any tag baked into the downstream message reached the client unfiltered.

Fix, localized to client.go (the single choke point all sn_*.go service calls funnel through via Client.Get/Patch/Post/Delete):

  • Added stripInternalErrorTag, which splits a message into a client-safe string (tag removed) and the tag itself (empty if none was present), matching a generic ^\[[A-Z][A-Z0-9_]*\]\s* pattern rather than hardcoding one tag name.
  • extractDownstreamMessage now uses this to build two things from the one input: it log.Printfs the tag + message for correlation, and returns only the tag-stripped message — the value that becomes the client-facing Msg.
  • No behavior change when no tag is present.

User stories

N/A — internal error-handling correctness fix, no user-facing feature.

Release note

Fix: error messages returned to the CSM portal no longer carry an internal [SOMETHING_ERROR]-style tag prefix; the underlying error text is preserved, the tag is now logged server-side only.

Documentation

N/A — internal error-handling implementation detail, no public API contract change.

Automation tests

  • Unit tests

    Added entity-service/internal/servicenow-integration-service/client_test.go: table-driven tests for stripInternalErrorTag (tag stripped, no-tag passthrough, lowercase-bracket non-tag left alone), plus an end-to-end test that spins up an httptest server returning a 409 with a [SERVICENOW_ERROR]-tagged message and asserts the apierror.ConflictError.Msg the client receives is tag-free.

  • Integration tests

    N/A — covered by the unit test exercising the real HTTP round trip through Client.Patch.

Security checks

Samples

N/A

Related PRs

None

Migrations (if applicable)

N/A — no schema or data change.

Test environment

Go (per entity-service/go.mod), macOS, go build ./..., go vet ./..., go test ./... — all pass.

Learning

N/A

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@rksk, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 26 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 87caaeb5-f746-4270-ac2b-368796eec79b

📥 Commits

Reviewing files that changed from the base of the PR and between e0049d1 and 9381c9c.

📒 Files selected for processing (2)
  • entity-service/internal/servicenow-integration-service/client.go
  • entity-service/internal/servicenow-integration-service/client_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@rksk

rksk commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Consolidated into #1264 to simplify review — no code changes, same commit content.

@rksk rksk closed this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant