diff --git a/projects/blog-site/src/runtime/app.ts b/projects/blog-site/src/runtime/app.ts index 304c939e9..0b6c92c59 100644 --- a/projects/blog-site/src/runtime/app.ts +++ b/projects/blog-site/src/runtime/app.ts @@ -40,8 +40,23 @@ export function createBlogApp( app.use(cookieParser()); app.use(createVisitorIdMiddleware({ generateVisitorId: deps.generateVisitorId, secure: deps.secureCookies })); - app.use(createClickAttributionMiddleware({ now: deps.now, secure: deps.secureCookies })); - app.use(createAnalyticsMiddleware({ logger: deps.analyticsLogger, salt: deps.salt, now: deps.now })); + + // The blog serves its shell assets from a separate static-asset origin (never + // through Express) and has no /view scheme-variant route, so no request path + // is ever a static asset and every landing path is already canonical. + const isStaticAssetPath = () => false; + const canonicalizeLandingPath = (path: string) => path; + app.use( + createClickAttributionMiddleware({ + now: deps.now, + secure: deps.secureCookies, + isStaticAssetPath, + canonicalizeLandingPath, + }), + ); + app.use( + createAnalyticsMiddleware({ logger: deps.analyticsLogger, salt: deps.salt, now: deps.now, isStaticAssetPath }), + ); const base = initBase(config); const blogPosts = initBlogPosts(); diff --git a/projects/hutch/Pulumi.prod.yaml b/projects/hutch/Pulumi.prod.yaml index f63b6859b..548ac15aa 100644 --- a/projects/hutch/Pulumi.prod.yaml +++ b/projects/hutch/Pulumi.prod.yaml @@ -76,6 +76,28 @@ config: b4248a499303636a, cae1bf1ec4181321, 41174e927f157da7, - 4b7117ffaf8a8d4c + 4b7117ffaf8a8d4c, + 275464cfd65e28a7, + b83406f00fd64b21, + 48c6982510e6bcc9, + 88594160212cdbdf, + f5921c92dcd21618, + f16b53a0519b3632, + 2860ed8349a9326a, + e9dd888c93de4608, + a4406141f80be403, + 43fb748de42b9620, + 06b078573bbaf6f3, + 573c2ff2b7939f2f, + a1814fdfe4229a14, + b4a154b234943575, + c754c936e8fdeec6, + 362e13462b08322b, + f9c1a522f591fcf1, + 3ed5982c96b1e184, + ced21dd8e16cf221, + 583b92ae3a8d2bea, + e39b171d9e2b7fdf, + ac278da65b1433b6 ] encryptionsalt: v1:l9Qbc2Rjk0w=:v1:atrW398hQVu5eMhu:NQ2XG9vAOXAa+XxkU2+ksKYFAJJLTA== diff --git a/projects/hutch/src/runtime/lambda.main.ts b/projects/hutch/src/runtime/lambda.main.ts index 0f707583e..d371d170b 100644 --- a/projects/hutch/src/runtime/lambda.main.ts +++ b/projects/hutch/src/runtime/lambda.main.ts @@ -7,6 +7,7 @@ import serverless from "serverless-http"; import { HutchLogger, consoleLogger } from "@packages/hutch-logger"; import { logger as requestLogger } from "./domain/logger"; import { createAnalyticsMiddleware, hashIp } from "@packages/web-analytics"; +import { isStaticAssetRequestPath } from "./web/static-asset-paths"; import { createBanMiddleware } from "./web/middleware/ban"; import { type BotBlockEvent, createBlockNaiveBotMiddleware } from "./web/middleware/naive-bot"; import { logAndRespondOnError } from "./web/middleware/error-handler"; @@ -28,6 +29,7 @@ const analytics = createAnalyticsMiddleware({ logger: analyticsLogger, salt, now: () => new Date(), + isStaticAssetPath: isStaticAssetRequestPath, }); const application = express() diff --git a/projects/hutch/src/runtime/server.ts b/projects/hutch/src/runtime/server.ts index be82666fc..546c41f44 100644 --- a/projects/hutch/src/runtime/server.ts +++ b/projects/hutch/src/runtime/server.ts @@ -135,6 +135,8 @@ import { import { initAuthRoutes } from "./web/auth/auth.page"; import type { BotDefenseEvent } from "./web/auth/auth.page"; import type { ConversionEvent } from "./conversions"; +import { CLIENT_DIST_MOUNT_PATH, isStaticAssetRequestPath } from "./web/static-asset-paths"; +import { canonicalizeViewLandingPath } from "./web/pages/view/view-path"; import { initGoogleAuthRoutes } from "./web/auth/google-auth.page"; import { initAppleAuthRoutes } from "./web/auth/apple-auth.page"; import { initResolveLogin } from "@packages/web-session"; @@ -427,13 +429,20 @@ export function createApp(dependencies: AppDependencies): Express { app.use(cookieParser()); app.use(changelogDismissMiddleware); app.use(createVisitorIdMiddleware({ generateVisitorId: randomUUID, secure: secureCookies })); - app.use(createClickAttributionMiddleware({ now: dependencies.now, secure: secureCookies })); + app.use( + createClickAttributionMiddleware({ + now: dependencies.now, + secure: secureCookies, + isStaticAssetPath: isStaticAssetRequestPath, + canonicalizeLandingPath: canonicalizeViewLandingPath, + }), + ); // Same-origin client bundles — the Lambda packaging step copies // src/runtime/web/client-dist/ into the bundle, so `__dirname/web/client-dist` // resolves both in dev (tsx → src/runtime/) and in prod (Lambda → /var/task/). app.use( - "/client-dist", + CLIENT_DIST_MOUNT_PATH, express.static(resolve(__dirname, "web", "client-dist"), { maxAge: "5m", fallthrough: false, diff --git a/projects/hutch/src/runtime/test-app.ts b/projects/hutch/src/runtime/test-app.ts index 584aac57d..3db9eeba9 100644 --- a/projects/hutch/src/runtime/test-app.ts +++ b/projects/hutch/src/runtime/test-app.ts @@ -26,6 +26,7 @@ import { useTestServer as useServerForFixture } from "@packages/web-test-harness import { createApp } from "./server"; import type { GetChangelogBanner } from "./web/changelog-banner-source"; import { initFoundingAllocation } from "./web/shared/founding-progress/founding-allocation"; +import { isStaticAssetRequestPath } from "./web/static-asset-paths"; import { type AnalyticsEvent, createAnalyticsMiddleware } from "@packages/web-analytics"; import { DEFAULT_INBOX_ALIAS } from "@packages/domain/inbox"; import type { UserId } from "@packages/domain/user"; @@ -248,6 +249,7 @@ export function createTestApp( logger: analyticsBundle.logger, salt: "test-analytics-salt", now: fixture.shared.now, + isStaticAssetPath: isStaticAssetRequestPath, })) .use(createApp({ ...flattenFixtureToAppDependencies(fixture, analyticsBundle), ...overrides })); return { diff --git a/projects/hutch/src/runtime/web/pages/view/view-path.test.ts b/projects/hutch/src/runtime/web/pages/view/view-path.test.ts index 9c64a7cc7..bc618ebec 100644 --- a/projects/hutch/src/runtime/web/pages/view/view-path.test.ts +++ b/projects/hutch/src/runtime/web/pages/view/view-path.test.ts @@ -1,4 +1,4 @@ -import { parseViewPath, viewPathFor } from "./view-path"; +import { canonicalizeViewLandingPath, parseViewPath, viewPathFor } from "./view-path"; function parse(decodedAndEncoded: string): ReturnType; function parse(args: { rawPath: string; encodedPath: string }): ReturnType; @@ -67,6 +67,40 @@ describe("viewPathFor", () => { }); }); +describe("canonicalizeViewLandingPath", () => { + it("collapses a /view/https:/ landing path to the scheme-less canonical", () => { + expect(canonicalizeViewLandingPath("/view/https:/fagnerbrack.com/learn-sql")).toBe( + "/view/fagnerbrack.com/learn-sql", + ); + }); + + it("collapses a /view/https:// landing path to the scheme-less canonical", () => { + expect(canonicalizeViewLandingPath("/view/https://fagnerbrack.com/learn-sql")).toBe( + "/view/fagnerbrack.com/learn-sql", + ); + }); + + it("keeps the explicit http:// scheme for a /view/http:/ landing path", () => { + expect(canonicalizeViewLandingPath("/view/http:/example.com/post")).toBe( + "/view/http://example.com/post", + ); + }); + + it("leaves an already-canonical /view path unchanged", () => { + expect(canonicalizeViewLandingPath("/view/fagnerbrack.com/learn-sql")).toBe( + "/view/fagnerbrack.com/learn-sql", + ); + }); + + it("leaves a non-/view path unchanged", () => { + expect(canonicalizeViewLandingPath("/queue")).toBe("/queue"); + }); + + it("leaves the root path unchanged", () => { + expect(canonicalizeViewLandingPath("/")).toBe("/"); + }); +}); + describe("parseViewPath", () => { it("treats a plain host/path as an https article", () => { expect(parse("example.com/post")).toEqual({ diff --git a/projects/hutch/src/runtime/web/pages/view/view-path.ts b/projects/hutch/src/runtime/web/pages/view/view-path.ts index 819b57274..192cf23e7 100644 --- a/projects/hutch/src/runtime/web/pages/view/view-path.ts +++ b/projects/hutch/src/runtime/web/pages/view/view-path.ts @@ -44,6 +44,18 @@ export function parseViewPath(input: ParseViewPathInput): ParseViewPathResult { return { kind: "render", articleUrl: `https://${rawPath}` }; } +/** Canonicalizes a `/view/://...` landing path to the same scheme-less + * (https) / literal-`http://` form `parseViewPath` redirects to, collapsing the + * `https:/` and `https://` variants that pollute click-attribution's + * `landing_path`. Used at cookie-set time, where the routing 301 cannot help + * because the cookie must be written before headers are sent. Non-`/view` + * scheme-bearing paths are returned unchanged. */ +export function canonicalizeViewLandingPath(path: string): string { + const match = /^\/view\/(https?):\/{1,2}(.+)$/i.exec(path); + if (!match) return path; + return match[1].toLowerCase() === "http" ? `/view/http://${match[2]}` : `/view/${match[2]}`; +} + /** Re-encode `%25` (literal `%`), `?`, and `#` so the canonical survives * Express's `decodeURIComponent` on the wildcard param. `%25` is the URL * constructor's encoding of a literal percent sign; double-encoding it to diff --git a/projects/hutch/src/runtime/web/static-asset-paths.test.ts b/projects/hutch/src/runtime/web/static-asset-paths.test.ts new file mode 100644 index 000000000..dcb880662 --- /dev/null +++ b/projects/hutch/src/runtime/web/static-asset-paths.test.ts @@ -0,0 +1,32 @@ +import { CLIENT_DIST_MOUNT_PATH, isStaticAssetRequestPath } from "./static-asset-paths"; + +describe("isStaticAssetRequestPath", () => { + it("exposes the /client-dist mount path as a single source of truth for server.ts", () => { + expect(CLIENT_DIST_MOUNT_PATH).toBe("/client-dist"); + }); + + it.each([ + "/client-dist", + "/client-dist/toast.client.js", + "/client-dist/view-paywall.client.js.map", + "/toast.client.js", + "/extension-suggestion-banner.client.js", + "/progress-bar.client.js.map", + "/apple-touch-icon.png", + "/apple-touch-icon-120x120-precomposed.png", + ])("classifies %s as a static asset", (path) => { + expect(isStaticAssetRequestPath(path)).toBe(true); + }); + + it.each([ + "/", + "/queue", + "/view/fagnerbrack.com/learn-sql-9aceb0bdee03", + "/view/example.com/image.png", + "/fagnerbrack.com/photo.png", + "/example.com", + "/https:/fagnerbrack.com/post", + ])("does not classify reader-shaped path %s as a static asset", (path) => { + expect(isStaticAssetRequestPath(path)).toBe(false); + }); +}); diff --git a/projects/hutch/src/runtime/web/static-asset-paths.ts b/projects/hutch/src/runtime/web/static-asset-paths.ts new file mode 100644 index 000000000..520f3ad4b --- /dev/null +++ b/projects/hutch/src/runtime/web/static-asset-paths.ts @@ -0,0 +1,21 @@ +export const CLIENT_DIST_MOUNT_PATH = "/client-dist"; + +const APPLE_TOUCH_ICON_PATH = /^\/apple-touch-icon(?:-\d+x\d+)?(?:-precomposed)?\.png$/; + +/** A one-segment `*.client.js` path can only be a mount-trimmed client-dist + * bundle: express.static responds terminally, so the router never restores the + * `/client-dist` prefix onto req.url. A reader permalink is always + * `//` (2+ segments) and no hostname ends in `.js`, so this shape is + * never a real page — a constraint the express types cannot express. */ +const TRIMMED_BUNDLE_PATH = /^\/[a-z0-9-]+\.client\.js(\.map)?$/; + +/** True when a request path targets a static asset served outside the page + * router (the /client-dist mount and the apple-touch-icon redirect in + * server.ts), so analytics and click-attribution can exclude it. Deliberately + * not a bare extension regex: reader paths mirror arbitrary source URLs and may + * end `.png`/`.html`. */ +export function isStaticAssetRequestPath(path: string): boolean { + if (path === CLIENT_DIST_MOUNT_PATH || path.startsWith(`${CLIENT_DIST_MOUNT_PATH}/`)) return true; + if (APPLE_TOUCH_ICON_PATH.test(path)) return true; + return TRIMMED_BUNDLE_PATH.test(path); +} diff --git a/src/packages/web-analytics/src/analytics.test.ts b/src/packages/web-analytics/src/analytics.test.ts index cabe24d87..b76e5fba1 100644 --- a/src/packages/web-analytics/src/analytics.test.ts +++ b/src/packages/web-analytics/src/analytics.test.ts @@ -51,21 +51,34 @@ function createRes(statusCode = 200): Response & EventEmitter { return emitter; } -function captureEvents(req: Partial, res: Response & EventEmitter): AnalyticsEvent[] { +function captureEvents( + req: Partial, + res: Response & EventEmitter, + beforeFinish?: () => void, + isStaticAssetPath: (path: string) => boolean = () => false, +): AnalyticsEvent[] { const { captured, logger } = createCapturingLogger(); const middleware = createAnalyticsMiddleware({ logger, salt: "test-salt", now: () => new Date("2026-04-21T10:00:00.000Z"), + isStaticAssetPath, }); const next: NextFunction = () => {}; middleware(req as Request, res, next); + beforeFinish?.(); res.emit("finish"); return captured; } -function runMiddleware(req: Partial, res: Response & EventEmitter): AnalyticsPageview[] { - return captureEvents(req, res).filter((e): e is AnalyticsPageview => e.event === "pageview"); +function runMiddleware( + req: Partial, + res: Response & EventEmitter, + isStaticAssetPath?: (path: string) => boolean, +): AnalyticsPageview[] { + return captureEvents(req, res, undefined, isStaticAssetPath).filter( + (e): e is AnalyticsPageview => e.event === "pageview", + ); } function runMiddlewareClicks(req: Partial, res: Response & EventEmitter): AnalyticsClick[] { @@ -195,6 +208,44 @@ describe("createAnalyticsMiddleware", () => { }); }); +describe("createAnalyticsMiddleware — asset & path hygiene", () => { + it("drops a request the injected isStaticAssetPath flags — asset classification is supplied by the host app (hutch's static routes), not built into the shared middleware", () => { + const events = runMiddleware( + createReq({ path: "/client-dist/toast.client.js" }), + createRes(200), + (path) => path.startsWith("/client-dist/"), + ); + expect(events).toEqual([]); + }); + + it("logs a path the injected isStaticAssetPath does not flag, even one ending in a file extension — extension alone never drops a pageview", () => { + const [event] = runMiddleware(createReq({ path: "/view/fagnerbrack.com/photo.png" }), createRes(200)); + expect(event.path).toBe("/view/fagnerbrack.com/photo.png"); + }); + + it("does not count a 301 redirect leg as a pageview — the destination logs its own", () => { + expect(runMiddleware(createReq({ path: "/view/https:/example.com/post" }), createRes(301))).toEqual([]); + }); + + it("does not count a 302 redirect leg as a pageview", () => { + expect(runMiddleware(createReq({ path: "/view" }), createRes(302))).toEqual([]); + }); + + it("does not count an informational 1xx response as a pageview", () => { + expect(runMiddleware(createReq({ path: "/queue" }), createRes(199))).toEqual([]); + }); + + it("snapshots req.path at middleware entry so a finish-time mount-trim mutation cannot corrupt the logged pageview path", () => { + const req = createReq({ path: "/view/fagnerbrack.com/learn-sql" }); + const res = createRes(200); + const pageviews = captureEvents(req, res, () => { + Object.defineProperty(req, "path", { value: "/fagnerbrack.com/learn-sql", configurable: true }); + }).filter((e): e is AnalyticsPageview => e.event === "pageview"); + expect(pageviews).toHaveLength(1); + expect(pageviews[0].path).toBe("/view/fagnerbrack.com/learn-sql"); + }); +}); + describe("createAnalyticsMiddleware — internal click events", () => { const internalQuery = { utm_source: "queue", utm_medium: "internal", utm_content: "subscribe" }; @@ -248,6 +299,12 @@ describe("createAnalyticsMiddleware — internal click events", () => { expect(runMiddleware(req, createRes(200))).toEqual([]); }); + it("still counts a click on a 303 redirect (POST-action → See Other) even though 3xx is no longer a pageview", () => { + const req = createReq({ method: "POST", path: "/queue/save", query: internalQuery }); + expect(runMiddlewareClicks(req, createRes(303))).toHaveLength(1); + expect(runMiddleware(req, createRes(303))).toEqual([]); + }); + it("does not count a click when the response is a 4xx/5xx error", () => { expect(runMiddlewareClicks(createReq({ query: internalQuery }), createRes(404))).toEqual([]); }); diff --git a/src/packages/web-analytics/src/analytics.ts b/src/packages/web-analytics/src/analytics.ts index 0feb848e5..d13e4e839 100644 --- a/src/packages/web-analytics/src/analytics.ts +++ b/src/packages/web-analytics/src/analytics.ts @@ -266,10 +266,16 @@ export type AnalyticsEvent = | ViewOpenedEvent | ViewSaveIntentEvent; -function shouldLog(params: { req: Request; path: string; statusCode: number }): boolean { +function shouldLog(params: { + req: Request; + path: string; + statusCode: number; + isStaticAssetPath: (path: string) => boolean; +}): boolean { if (params.req.method !== "GET") return false; if (SKIP_PATHS.has(params.path)) return false; - if (params.statusCode >= 400) return false; + if (params.isStaticAssetPath(params.path)) return false; + if (params.statusCode < 200 || params.statusCode >= 300) return false; if (isbot(params.req.get("user-agent"))) return false; if (params.req.get("hx-request") === "true") return false; return true; @@ -403,6 +409,7 @@ export function createAnalyticsMiddleware(deps: { logger: HutchLogger.Typed; salt: string; now: () => Date; + isStaticAssetPath: (path: string) => boolean; }): RequestHandler { return (req: Request, res: Response, next: NextFunction) => { /** Capture the request path up front. A sub-router mount (e.g. blog-site's @@ -426,7 +433,7 @@ export function createAnalyticsMiddleware(deps: { is_authenticated: req.userId ? 1 : 0, }); } - if (!shouldLog({ req, path, statusCode: res.statusCode })) return; + if (!shouldLog({ req, path, statusCode: res.statusCode, isStaticAssetPath: deps.isStaticAssetPath })) return; const userAgent = req.get("user-agent"); deps.logger.info({ stream: STREAMS.analytics, diff --git a/src/packages/web-analytics/src/click-attribution.middleware.test.ts b/src/packages/web-analytics/src/click-attribution.middleware.test.ts index e1ed6d1dd..86b68c223 100644 --- a/src/packages/web-analytics/src/click-attribution.middleware.test.ts +++ b/src/packages/web-analytics/src/click-attribution.middleware.test.ts @@ -49,12 +49,18 @@ function createRes(): { res: Partial; cookies: CapturedCookie[] } { function runMiddleware( req: Partial, - options: { secure: boolean } = { secure: false }, + options: { + secure?: boolean; + isStaticAssetPath?: (path: string) => boolean; + canonicalizeLandingPath?: (path: string) => string; + } = {}, ): { cookies: CapturedCookie[]; nextCalled: boolean } { const { res, cookies } = createRes(); const middleware = createClickAttributionMiddleware({ now: () => new Date("2026-05-13T10:00:00.000Z"), - secure: options.secure, + secure: options.secure ?? false, + isStaticAssetPath: options.isStaticAssetPath ?? (() => false), + canonicalizeLandingPath: options.canonicalizeLandingPath ?? ((path) => path), }); let nextCalled = false; const next: NextFunction = () => { @@ -162,6 +168,37 @@ describe("createClickAttributionMiddleware", () => { }); }); + it("does not mint a cookie when the injected isStaticAssetPath flags the path, so asset fetches never steal first-touch attribution", () => { + const req = createReq({ path: "/client-dist/toast.client.js" }); + const { cookies, nextCalled } = runMiddleware(req, { + isStaticAssetPath: (path) => path.startsWith("/client-dist/"), + }); + + expect(nextCalled).toBe(true); + expect(cookies).toEqual([]); + }); + + it("lets the next non-asset page win first-touch after a flagged asset request minted no cookie", () => { + const isStaticAssetPath = (path: string) => path.startsWith("/client-dist/"); + expect(runMiddleware(createReq({ path: "/client-dist/toast.client.js" }), { isStaticAssetPath }).cookies).toEqual( + [], + ); + const { cookies } = runMiddleware(createReq({ path: "/view/fagnerbrack.com/learn-sql" }), { isStaticAssetPath }); + + expect(cookies).toHaveLength(1); + expect(parseCookieValue(cookies[0].value).landing_path).toBe("/view/fagnerbrack.com/learn-sql"); + }); + + it("stores the injected canonicalizeLandingPath's output as landing_path so scheme variants of the same article no longer split attribution", () => { + const req = createReq({ path: "/view/https:/fagnerbrack.com/learn-sql" }); + const { cookies } = runMiddleware(req, { + canonicalizeLandingPath: (path) => path.replace("/view/https:/", "/view/"), + }); + + expect(cookies).toHaveLength(1); + expect(parseCookieValue(cookies[0].value).landing_path).toBe("/view/fagnerbrack.com/learn-sql"); + }); + it("skips non-GET requests so form posts can never reset the first-touch cookie", () => { const req = createReq({ method: "POST", diff --git a/src/packages/web-analytics/src/click-attribution.middleware.ts b/src/packages/web-analytics/src/click-attribution.middleware.ts index 50cfa7db0..c0d345d40 100644 --- a/src/packages/web-analytics/src/click-attribution.middleware.ts +++ b/src/packages/web-analytics/src/click-attribution.middleware.ts @@ -58,6 +58,8 @@ export function readClickAttribution(req: Request): ClickAttribution | undefined export function createClickAttributionMiddleware(deps: { now: () => Date; secure: boolean; + isStaticAssetPath: (path: string) => boolean; + canonicalizeLandingPath: (path: string) => string; }): RequestHandler { const cookieOptions = { ...baseCookieOptions(deps.secure), maxAge: CLICK_COOKIE_MAX_AGE_MS }; return (req: Request, res: Response, next: NextFunction) => { @@ -69,6 +71,10 @@ export function createClickAttributionMiddleware(deps: { next(); return; } + if (deps.isStaticAssetPath(req.path)) { + next(); + return; + } if (readClickAttribution(req)) { next(); return; @@ -86,7 +92,7 @@ export function createClickAttributionMiddleware(deps: { * no-attribution cookie minimal. */ const attribution: ClickAttribution = { first_seen_at: deps.now().toISOString(), - landing_path: req.path, + landing_path: deps.canonicalizeLandingPath(req.path), ...(utm_source ? { utm_source } : {}), ...(utm_medium ? { utm_medium } : {}), ...(utm_campaign ? { utm_campaign } : {}),