Skip to content

Commit 8165082

Browse files
fix: address PR review comments on App Insights telemetry
- appinsights-manager.js: ecsGetCorrelationContext() now returns a full W3C traceparent (00-{traceId}-{spanId}-01) instead of a bare 32-hex traceId, removing ambiguity for callers - appinsights-manager.js: script.remove() on onerror and on 15s timeout so the dead element is cleaned up and a subsequent retry appends a fresh <script> element rather than waiting another 15s - _Layout.cshtml: replace window.AUTHENTICATED_USER_ID global with a <meta name='ecs-auth-user-id'> tag, scoped to avoid exposing the stable user GUID to third-party scripts that enumerate window globals - appinsights-manager.js: getAuthenticatedUserId() reads from meta tag - Program.cs: validate ingestionUri.Scheme == https before emitting into CSP header to guard against malformed connection strings - Program.cs: remove legacy dc.services.visualstudio.com from connect-src (modern resources use *.in.applicationinsights.azure.com)
1 parent f342f7a commit 8165082

3 files changed

Lines changed: 41 additions & 10 deletions

File tree

EssentialCSharp.Web/Program.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ await McpJsonRpcResponseWriter.WriteErrorAsync(
512512
$"style-src 'self' 'unsafe-inline' cdnjs.cloudflare.com fonts.googleapis.com https://hcaptcha.com https://*.hcaptcha.com",
513513
$"font-src 'self' fonts.gstatic.com cdnjs.cloudflare.com",
514514
$"img-src 'self' data: https:",
515-
$"connect-src 'self' https://hcaptcha.com https://*.hcaptcha.com https://api.pwnedpasswords.com https://*.algolia.net https://*.algolianet.com https://*.google-analytics.com https://*.clarity.ms https://dc.services.visualstudio.com https://*.in.applicationinsights.azure.com{GetApplicationInsightsCspSources(app.Configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"])}{tryDotNetSources}",
515+
$"connect-src 'self' https://hcaptcha.com https://*.hcaptcha.com https://api.pwnedpasswords.com https://*.algolia.net https://*.algolianet.com https://*.google-analytics.com https://*.clarity.ms https://*.in.applicationinsights.azure.com{GetApplicationInsightsCspSources(app.Configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"])}{tryDotNetSources}",
516516
$"frame-src https://hcaptcha.com https://*.hcaptcha.com https://newassets.hcaptcha.com{tryDotNetSources}",
517517
$"worker-src blob:",
518518
$"frame-ancestors 'none'",
@@ -678,7 +678,9 @@ private static string GetApplicationInsightsCspSources(string? connectionString)
678678
}
679679

680680
string? ingestionEndpoint = GetConnectionStringValue(connectionString, "IngestionEndpoint");
681-
if (string.IsNullOrWhiteSpace(ingestionEndpoint) || !Uri.TryCreate(ingestionEndpoint, UriKind.Absolute, out Uri? ingestionUri))
681+
if (string.IsNullOrWhiteSpace(ingestionEndpoint)
682+
|| !Uri.TryCreate(ingestionEndpoint, UriKind.Absolute, out Uri? ingestionUri)
683+
|| ingestionUri.Scheme != Uri.UriSchemeHttps)
682684
{
683685
return string.Empty;
684686
}

EssentialCSharp.Web/Views/Shared/_Layout.cshtml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,15 @@
5555
<!-- Cookie Consent Manager - Load before analytics -->
5656
<script src="~/js/consent-manager.js" asp-append-version="true"></script>
5757
<script src="~/js/appinsights-manager.js" asp-append-version="true"></script>
58+
@{
59+
string? authUserId = User.FindFirstValue(ClaimTypes.NameIdentifier);
60+
}
61+
@if (!string.IsNullOrEmpty(authUserId))
62+
{
63+
// Scoped to a <meta> tag rather than a window global to avoid exposing the stable
64+
// user GUID to third-party scripts that enumerate window properties.
65+
<meta name="ecs-auth-user-id" content="@authUserId" />
66+
}
5867

5968
<!-- Microsoft Clarity - Will be activated based on consent -->
6069
<script type="text/javascript">
@@ -193,7 +202,6 @@
193202
window.IS_AUTHENTICATED = @Json.Serialize(SignInManager.IsSignedIn(User));
194203
window.TRYDOTNET_ORIGIN = @Json.Serialize(Configuration["TryDotNet:Origin"]);
195204
window.APPLICATIONINSIGHTS_CONNECTION_STRING = @Json.Serialize(Configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"]);
196-
window.AUTHENTICATED_USER_ID = @Json.Serialize(User.FindFirstValue(ClaimTypes.NameIdentifier));
197205
window.BUILD_LABEL = @Json.Serialize(buildLabel);
198206
window.ENABLE_CHAT_WIDGET = @Json.Serialize(!Context.Request.Path.StartsWithSegments("/Identity"));
199207
</script>

EssentialCSharp.Web/wwwroot/js/appinsights-manager.js

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,12 @@
2525
}
2626

2727
function getAuthenticatedUserId() {
28-
const userId = window.AUTHENTICATED_USER_ID;
29-
return typeof userId === "string" && userId.trim().length > 0 ? userId.trim() : null;
28+
// Read from a <meta> tag rather than a window global to avoid exposing the stable
29+
// user GUID to third-party scripts that enumerate window properties.
30+
const meta = document.querySelector('meta[name="ecs-auth-user-id"]');
31+
if (!meta) { return null; }
32+
const value = meta.getAttribute("content") || "";
33+
return value.trim().length > 0 ? value.trim() : null;
3034
}
3135

3236
function setAuthenticatedContext() {
@@ -64,13 +68,20 @@
6468
resolve();
6569
return;
6670
}
67-
// Guard: script may have already errored — add timeout so promise doesn't hang forever
71+
// Guard: script may have already errored — add timeout so promise doesn't hang forever.
72+
// On timeout, remove the dead element so the next retry can append a fresh one.
6873
const timeoutId = setTimeout(() => {
6974
sdkLoadPromise = null;
75+
existing.remove();
7076
reject(new Error("App Insights SDK load timed out."));
7177
}, 15000);
7278
existing.addEventListener("load", () => { clearTimeout(timeoutId); resolve(); }, { once: true });
73-
existing.addEventListener("error", () => { clearTimeout(timeoutId); sdkLoadPromise = null; reject(new Error("Failed to load App Insights SDK.")); }, { once: true });
79+
existing.addEventListener("error", () => {
80+
clearTimeout(timeoutId);
81+
sdkLoadPromise = null;
82+
existing.remove(); // remove so the next retry appends a fresh element
83+
reject(new Error("Failed to load App Insights SDK."));
84+
}, { once: true });
7485
return;
7586
}
7687

@@ -81,6 +92,7 @@
8192
script.onload = () => resolve();
8293
script.onerror = () => {
8394
sdkLoadPromise = null; // allow retry on transient failure
95+
script.remove(); // remove dead element so the next retry appends a fresh one
8496
reject(new Error("Failed to load App Insights SDK."));
8597
};
8698
document.head.appendChild(script);
@@ -173,10 +185,17 @@
173185
}
174186
}
175187

176-
function getCurrentTraceId() {
188+
function generateSpanId() {
189+
const arr = new Uint8Array(8);
190+
crypto.getRandomValues(arr);
191+
return Array.from(arr, function (b) { return b.toString(16).padStart(2, "0"); }).join("");
192+
}
193+
194+
function getCurrentTraceparent() {
177195
const traceId = appInsights?.context?.telemetryTrace?.traceID;
178196
if (typeof traceId === "string" && /^[a-f0-9]{32}$/i.test(traceId)) {
179-
return traceId.toLowerCase();
197+
// Return a full W3C traceparent so callers don't need to synthesise span IDs.
198+
return `00-${traceId.toLowerCase()}-${generateSpanId()}-01`;
180199
}
181200
return null;
182201
}
@@ -185,8 +204,10 @@
185204
return appInsights;
186205
};
187206

207+
// Returns a W3C traceparent string (00-{traceId}-{spanId}-01) suitable for passing
208+
// as configuration.correlationContext to the TryDotNet SDK.
188209
window.ecsGetCorrelationContext = function () {
189-
return getCurrentTraceId();
210+
return getCurrentTraceparent();
190211
};
191212

192213
function init() {

0 commit comments

Comments
 (0)