From bd46b7214469e3b5fa59302a4fa98fdf4d92be26 Mon Sep 17 00:00:00 2001
From: Roman Priesol <roman@priesol.net>
Date: Wed, 16 Sep 2026 17:56:44 +0200
Subject: [PATCH] Skip re-writing the session cookie on every response, not
 just the three that change it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Rack::Session::Cookie re-serializes and re-sets the cookie on every
response that even reads session — Rack::Protection's CSRF check does,
on nearly every request — reflecting whatever cookie that particular
request happened to arrive with, whether or not anything actually
changed. A HAR capture caught the real failure mode: seconds after a
successful login, offline_data.js's _checkUpdate (an unrelated
background poll fired the instant the page loads) was still carrying
the browser's pre-login cookie — a client-side timing race across the
OAuth redirect chain, not something this server controls — and its own
re-issued Set-Cookie landed after the login's, silently overwriting
session[:user] with nothing logged anywhere to explain it.

Only /login, /auth/callback and /logout ever actually write to
session. A before filter now skips the write everywhere else, removing
every other response from that race — a stale request can still read a
stale cookie, but it can no longer clobber a fresher one.

Also drops the earlier except: [:session_hijacking] from the previous
commit: rack-protection 4.2.1 only enables SessionHijacking when
explicitly opted into via :use, so Sinatra's default setup never turned
it on in the first place — the exclusion was a no-op, not the fix.
---
 app.rb | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/app.rb b/app.rb
index 0845779..b4a7a49 100644
--- a/app.rb
+++ b/app.rb
@@ -34,21 +34,6 @@ module Maptool
     set :sessions, httponly: true, secure: production?, same_site: :lax,
                    secret: ENV.fetch("SESSION_SECRET") { SecureRandom.hex(32) }
 
-    # Sinatra enables Rack::Protection::SessionHijacking by default once
-    # sessions are on: it fingerprints a session to the User-Agent header
-    # seen on the request that first wrote to it, and *silently* empties
-    # the session (no exception, nothing logged) on any later request
-    # where that header doesn't match — which is exactly what a login
-    # behind production's reverse proxy hit: the OAuth callback set
-    # session[:user] fine, but the very next ordinary page load (or even a
-    # plain refresh) came back logged out with nothing in the logs to
-    # explain it. Its own docs already admit every header it can check is
-    # spoofable anyway ("will not prevent determined hijacking attempts")
-    # — losing it is a minor tradeoff for a login that actually stays
-    # logged in. Every other Rack::Protection check (XSS header, path
-    # traversal, JSON CSRF, frame options, ...) stays on.
-    set :protection, except: [:session_hijacking]
-
     # Cache-busts /js and /css requests: browsers (and a caching proxy in
     # front of production, see docs/deployment.adoc) hang onto static
     # assets for hours, so without this a deploy that changes app.js/css
@@ -165,6 +150,28 @@ module Maptool
       end
     end
 
+    # Only these three routes ever write to session (oauth_state, user,
+    # session.clear below) — every other route only ever reads it, if it
+    # touches session at all. That still matters: Rack::Session::Cookie
+    # re-serializes and re-sets the cookie on *every* response that so
+    # much as read session (Rack::Protection's own CSRF check does, on
+    # nearly every request) — reflecting whatever cookie that particular
+    # request happened to arrive with, whether or not anything changed.
+    # A HAR capture caught the actual failure mode this caused: seconds
+    # after a successful login, offline_data.js's _checkUpdate — an
+    # unrelated background poll fired the instant the page loads — was
+    # still carrying the browser's pre-login cookie (a client-side timing
+    # race across the OAuth redirect chain, not something this server
+    # controls), and its own re-issued Set-Cookie landed *after* the
+    # login's, silently overwriting session[:user] back out with nothing
+    # logged anywhere to explain it. Skipping the write everywhere except
+    # the routes that actually need it removes every other response from
+    # that race — a stale request can still be served from a stale
+    # cookie, but it can no longer clobber a fresher one.
+    before do
+      session.options[:skip] = true unless %w[/login /auth/callback /logout].include?(request.path_info)
+    end
+
     get "/login" do
       session[:oauth_state] = SecureRandom.hex(16)
       redirect OAUTH_CLIENT.auth_code.authorize_url(