X-Forwarded-For

[BayerC]

DO NOT MERGE

https://opencups-test-x-forwarded-for.streamlit.app/

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (19578fb) to head (1d90f1a).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #175   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          529       532    +3     
=========================================
+ Hits           529       532    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• Consider handling the common case where X-Forwarded-For contains a comma-separated list of IPs (e.g., by splitting on , and taking the first non-empty value) instead of using the raw header string.
• Depending on how st.context.headers is populated, it may be safer to guard against it being None (e.g., headers = getattr(st.context, 'headers', {})) to avoid runtime errors in non-HTTP or local contexts.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider handling the common case where `X-Forwarded-For` contains a comma-separated list of IPs (e.g., by splitting on `,` and taking the first non-empty value) instead of using the raw header string.
- Depending on how `st.context.headers` is populated, it may be safer to guard against it being `None` (e.g., `headers = getattr(st.context, 'headers', {})`) to avoid runtime errors in non-HTTP or local contexts.

## Individual Comments

### Comment 1
<location path="src/open_cups/app.py" line_range="261" />
<code_context>

 def run() -> None:
+    headers = st.context.headers
+    ip_address = headers.get("X-Forwarded-For", "Unknown")
+
+    st.write(f"Connecting from: {ip_address}")
</code_context>
<issue_to_address>
**suggestion:** Consider parsing `X-Forwarded-For` correctly when it contains a list of IPs.

Many proxies send `X-Forwarded-For` as `client-ip, proxy1, proxy2, ...`. Using it directly may log the whole chain instead of the client IP. If you want the originating client IP, split on `","` and use the first trimmed entry.
</issue_to_address>

### Comment 2
<location path="src/open_cups/app.py" line_range="263" />
<code_context>
+    headers = st.context.headers
+    ip_address = headers.get("X-Forwarded-For", "Unknown")
+
+    st.write(f"Connecting from: {ip_address}")
     st_autorefresh(interval=AUTOREFRESH_INTERVAL_MS, key="data_refresh")

</code_context>
<issue_to_address>
**🚨 question (security):** Double-check whether exposing the client IP in the UI aligns with privacy/user expectations.

Depending on your context, this could be treated as sensitive information. If it’s mainly for diagnostics, consider hiding it behind a debug flag or limiting visibility to admin/privileged users.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion: Consider parsing X-Forwarded-For correctly when it contains a list of IPs.

Many proxies send X-Forwarded-For as client-ip, proxy1, proxy2, .... Using it directly may log the whole chain instead of the client IP. If you want the originating client IP, split on "," and use the first trimmed entry.

[sourcery-ai]

🚨 question (security): Double-check whether exposing the client IP in the UI aligns with privacy/user expectations.

Depending on your context, this could be treated as sensitive information. If it’s mainly for diagnostics, consider hiding it behind a debug flag or limiting visibility to admin/privileged users.