Skip to content

Fix: Unauthenticated /api/tracking endpoints expose client IP addresses and request metadata - #311

Open
WesternConcrete wants to merge 4 commits into
masterfrom
devin/1788073896-gate-api-tracking-endpoints
Open

WesternConcrete wants to merge 4 commits into
masterfrom
devin/1788073896-gate-api-tracking-endpoints

Conversation

@WesternConcrete

@WesternConcrete WesternConcrete commented Aug 30, 2026

Copy link
Copy Markdown

Summary

Finding: Unauthenticated /api/tracking endpoints expose client IP addresses and request metadata (COMPLIANCE / NS terms — data egress + access path) in COG-GTM/ftgo-monolith.

The app ships no Spring Security filter chain, so all five /api/tracking read endpoints were reachable by any caller and returned per-request client IPs, query strings (consumer/order identifiers) and User-Agents from api_request_log. Fix closes both the access path and the retention of the personal data itself:

@RestController @RequestMapping("/api/tracking")
+ @ConditionalOnProperty(name = "ftgo.api-tracking.logs-endpoint.enabled", havingValue = "true")

// ApiTrackingInterceptor.preHandle
- request.getQueryString(), request.getRemoteAddr()
+ redactQueryString(request.getQueryString()),  // "consumerId=REDACTED&limit=REDACTED"
+ anonymizeAddress(request.getRemoteAddr())     // "10.1.2.37" -> "10.1.2.0"

The controller is now absent from the bean graph unless the deployment explicitly opts in, and the interceptor keeps its diagnostic value (parameter names, network prefix, latency, status) without retaining caller-identifying data. Verified with ./gradlew :ftgo-common:compileJava.

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


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Devin Review (Staging)
Devin Review

…ery values

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 3 potential issues.

Devin Review


@RestController
@RequestMapping(path = "/api/tracking")
@ConditionalOnProperty(name = "ftgo.api-tracking.logs-endpoint.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 logs remain publicly readable

When ftgo.api-tracking.logs-endpoint.enabled is true, every caller can read stored request metadata. The opt-in restores the original data-exposure path without access control.

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.

Correct, and intentional for this PR: the repo has no Spring Security dependency or filter chain anywhere, so real authorization on /api/tracking means introducing an auth stack for the whole monolith — larger than this remediation. This PR removes the default exposure path (endpoints absent from the bean graph unless explicitly enabled) and strips the caller-identifying fields from what those endpoints could return. Happy to follow up with a Spring Security PR gating /api/tracking/** if you want that scope.

Comment on lines +43 to 45
redactQueryString(request.getQueryString()),
anonymizeAddress(request.getRemoteAddr()),
request.getHeader("User-Agent")

@devin-ai-integration devin-ai-integration Bot Aug 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟨 User agents remain stored verbatim

Every request still persists the caller-supplied User-Agent value. Logs retain identifying client metadata and any sensitive text placed in that header.

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.

Kept deliberately: User-Agent identifies the client software, not the individual, and it is the remaining field that makes the request log useful for triage once the IP is anonymized and query values are redacted. The NS-relevant identifiers (client IP, consumer/order ids in query strings) are the ones this PR removes. Say the word if you want the header dropped or truncated too and I'll add it.

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>
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