Add "source" field to user-agent header#3
Conversation
![vibeops-ai[bot]](https://avatars.githubusercontent.com/in/1533038?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ VibeOps Review — The PR has one blocker: the new 'source' field bypasses the telemetry opt-out mechanism, potentially violating user privacy expectations.
🟡 Human review required · verdict=request_changes
Blockers (1)
Source field bypasses telemetry opt-out mechanism (confirmed by logic, skeptic_agreed)
src/main/java/com/stripe/net/HttpClient.java:279
The UNAME_HASH is included in the X-Stripe-Client-User-Agent header unconditionally, even when users set Stripe.enableTelemetry = false to opt out of system information collection. The platform field correctly respects this setting (lines 261-269), but the source field does not. This creates inconsistent privacy behavior where users who explicitly disable telemetry still have their system fingerprint (OS, Java version, hostname hash) transmitted. Fix: wrap the source field insertion with the same telemetry guard: if (Stripe.enableTelemetry && !UNAME_HASH.isEmpty()) { propertyMap.put("source", UNAME_HASH); }
if (Stripe.enableTelemetry && !UNAME_HASH.isEmpty()) {
propertyMap.put("source", UNAME_HASH);
}
What's good
- Good defensive programming with comprehensive exception handling in computeUnameHash() and getHostname() methods, ensuring the system degrades gracefully if system properties or hostname resolution fails.
- Well-structured test coverage that validates both the presence of the source field and the edge case where UNAME_HASH is empty, demonstrating thorough consideration of different runtime scenarios.
Context
- The PR description mentions this is a partial restoration of system information that was previously removed for privacy reasons, with tech support requesting some data back for debugging. The telemetry opt-out mechanism appears to be the intended privacy control.
Reviewed by VibeOps
| propertyMap.put("ai_agent", aiAgent); | ||
| } | ||
|
|
||
| if (!UNAME_HASH.isEmpty()) { |
There was a problem hiding this comment.
🛑 Blocker · Security · The UNAME_HASH is included in the X-Stripe-Client-User-Agent header unconditionally, even when users set Stripe.enableTelemetry = false to opt out of system information collection. The platform field correctly respects this setting (lines 261-269), but the source field does not. This creates inconsistent privacy behavior where users who explicitly disable telemetry still have their system fingerprint (OS, Java version, hostname hash) transmitted. Fix: wrap the source field insertion with the same telemetry guard: if (Stripe.enableTelemetry && !UNAME_HASH.isEmpty()) { propertyMap.put("source", UNAME_HASH); }
🔗 Backed by: agent reasoning
| if (!UNAME_HASH.isEmpty()) { | |
| if (Stripe.enableTelemetry && !UNAME_HASH.isEmpty()) { | |
| propertyMap.put("source", UNAME_HASH); | |
| } |
Was this helpful? React with 👍 or 👎 to help VibeOps learn.
There was a problem hiding this comment.
⚠️ VibeOps Review — The source field bypasses the telemetry opt-out flag, creating a privacy inconsistency that must be fixed.
🟡 Human review required · verdict=request_changes
Blockers (1)
Source field bypasses telemetry opt-out flag (confirmed by logic)
src/main/java/com/stripe/net/HttpClient.java:279
The source field (MD5 hash including hostname) is added to X-Stripe-Client-User-Agent unconditionally, while the platform field (OS info only) respects Stripe.enableTelemetry. When users set Stripe.enableTelemetry = false to opt out of telemetry, they expect no system information to be transmitted, but the hostname-derived hash still gets sent. Fix: wrap the source field addition in if (Stripe.enableTelemetry) like the platform field at lines 261-268.
if (Stripe.enableTelemetry && !UNAME_HASH.isEmpty()) {
propertyMap.put("source", UNAME_HASH);
}
💭 Heads-up · new code, no historical precedent (1)
Agent-confident findings on patterns we haven't seen reviewed before in this repo. Not blocking. Skim, ignore, or address.
UNAME_HASH is declared as a non-final, non-volatile package-private static field (static String UNAME_HASH)
src/main/java/com/stripe/net/HttpClient.java:30
UNAME_HASH is declared as a non-final, non-volatile package-private static field (static String UNAME_HASH). This has two problems: (1) Without volatile, reads/writes from multiple threads are not guaranteed to see a consistent value — a thread could observe a partially-initialized or stale value. (2) The field is mutable by any code in the same package (as demonstrated by the test directly assigning HttpClient.UNAME_HASH = ""), making it easy to accidentally corrupt shared state. Since the value is computed once at class-load time and should never change, it should be declared static final. The test that mutates it should instead use a test-specific override mechanism (e.g., a package-private setter or reflection) rather than relying on the field being mutable.
🔗 Backed by: agent reasoning
What's good
- Good test coverage for the new source field functionality, including both positive and negative cases.
- Clean implementation of MD5 hashing with proper exception handling and fallback to empty string.
Context
- The PR description mentions re-adding system information for tech support debugging after privacy concerns led to its removal earlier this year.
Reviewed by VibeOps
| propertyMap.put("ai_agent", aiAgent); | ||
| } | ||
|
|
||
| if (!UNAME_HASH.isEmpty()) { |
There was a problem hiding this comment.
🛑 Blocker · Security · The source field (MD5 hash including hostname) is added to X-Stripe-Client-User-Agent unconditionally, while the platform field (OS info only) respects Stripe.enableTelemetry. When users set Stripe.enableTelemetry = false to opt out of telemetry, they expect no system information to be transmitted, but the hostname-derived hash still gets sent. Fix: wrap the source field addition in if (Stripe.enableTelemetry) like the platform field at lines 261-268.
🔗 Backed by: agent reasoning
| if (!UNAME_HASH.isEmpty()) { | |
| if (Stripe.enableTelemetry && !UNAME_HASH.isEmpty()) { | |
| propertyMap.put("source", UNAME_HASH); | |
| } |
Was this helpful? React with 👍 or 👎 to help VibeOps learn.
| /** A value indicating whether the client should sleep between automatic request retries. */ | ||
| boolean networkRetriesSleep = true; | ||
|
|
||
| static String UNAME_HASH = computeUnameHash(); |
There was a problem hiding this comment.
💭 Heads-up · Bug · UNAME_HASH is declared as a non-final, non-volatile package-private static field (static String UNAME_HASH). This has two problems: (1) Without volatile, reads/writes from multiple threads are not guaranteed to see a consistent value — a thread could observe a partially-initialized or stale value. (2) The field is mutable by any code in the same package (as demonstrated by the test directly assigning HttpClient.UNAME_HASH = ""), making it easy to accidentally corrupt shared state. Since the value is computed once at class-load time and should never change, it should be declared static final. The test that mutates it should instead use a test-specific override mechanism (e.g., a package-private setter or reflection) rather than relying on the field being mutable.
No historical precedent in your repo — agent reasoning only. Not blocking. Skim, ignore, or address.
🔗 Backed by: agent reasoning
| static String UNAME_HASH = computeUnameHash(); | |
| static final String UNAME_HASH = computeUnameHash(); |
Was this helpful? React with 👍 or 👎 to help VibeOps learn.
2 participants
Why?
Earlier this year, we removed the output of
unamefrom the user-agent header for privacy reasons. It over-collected data we usually didn't need.Since then, our tech support team has reached out asking us to re-add some of the information to help with complex debugging tasks: sometimes they need to help users identify exactly which machine produced an API call.
Rather than just revert the previous change and reintroduce the same privacy issues that caused its removal in the first place, we're taking a more privacy-focused approach. Rather than send all the PII in plaintext, we hash it first. This way it can still be used to confirm the source of an API call if a user generates the hashes themselves, but isn't sensitive in and of itself.
What?
sourcekey to user-agent hash. It's contents are an md5 hash ofuname -aSee Also
Mirrored from stripe/stripe-java#2230 by
xavdidfor the VibeOps live demo. Real PR, real diff, reviewed by the same pipeline our customers run.