Skip to content

Fix: Unauthenticated /api/tracking endpoints expose retained request telemetry including client IP addresses - #315

Open
WesternConcrete wants to merge 3 commits into
masterfrom
devin/1788160211-ns-gate-api-tracking-endpoints
Open

WesternConcrete wants to merge 3 commits into
masterfrom
devin/1788160211-ns-gate-api-tracking-endpoints

Conversation

@WesternConcrete

@WesternConcrete WesternConcrete commented Aug 31, 2026

Copy link
Copy Markdown

Summary

Finding: Unauthenticated /api/tracking endpoints expose retained request telemetry including client IP addresses (NS terms / data-flow gap) in COG-GTM/ftgo-monolith.

ApiTrackingInterceptor persisted remoteAddr, the full queryString and User-Agent for every request to /**, and ApiTrackingController re-published those rows over unauthenticated /api/tracking/* with unbounded lookback (the app has no security starter or WebSecurityConfigurer).

Fix approach: close the exposure and shrink what is retained —

// ApiTrackingController
@ConditionalOnProperty(name = "ftgo.api-tracking.endpoints.enabled", havingValue = "true")

// ApiTrackingInterceptor.preHandle
- request.getQueryString(), request.getRemoteAddr()
+ null, anonymize(request.getRemoteAddr())   // last IPv4 octet / IPv6 suffix zeroed

ftgo.api-tracking.endpoints.enabled=false is set explicitly in application.properties so the boundary is visible in config; operators that need the read API must opt in (behind their own access control). Request/response timing, status and correlation-ID tracking are unchanged.

Link to Devin session: https://app.devin.ai/sessions/77249313a8fd4c14b8ebc931630a71f3
Open in Devin Desktop: https://app.devin.ai/desktop/session/77249313a8fd4c14b8ebc931630a71f3?variant=devin
Requested by: @WesternConcrete


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Devin Review (Staging)
Devin Review

…ent data

Co-Authored-By: Wes Convery <2wconvery@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 2 potential issues.

Devin Review


@RestController
@RequestMapping(path = "/api/tracking")
@ConditionalOnProperty(name = "ftgo.api-tracking.endpoints.enabled", havingValue = "true")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟨 Opt-in exposes telemetry without authentication

Enabling ftgo.api-tracking.endpoints.enabled publishes retained request metadata to every network client. Operators cannot enable diagnostics safely without separate perimeter controls.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agreed that the opt-in path is still unauthenticated, but adding authentication is out of scope for this minimal remediation: the application has no Spring Security (or any auth) dependency today, so introducing one would change the security model of every endpoint in the monolith, not just /api/tracking.

What this PR does close: the endpoints are off by default (ftgo.api-tracking.endpoints.enabled=false), and the persisted telemetry no longer contains query strings or full client IPs, so even when an operator opts in the exposed data is coarsened. Gating diagnostics behind authentication would be the right follow-up — flagging it for the repo owners rather than bundling an auth stack into this fix.

Co-Authored-By: Wes Convery <2wconvery@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

Co-Authored-By: Wes Convery <2wconvery@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant