fix(web): make the answer to an invalid cookie uncacheable

withSession clears a cookie it rejects, but the page then went out with a
public Cache-Control and no Vary, so a shared cache could keep an answer
tied to a cookie. Set no-store and Vary: Cookie when clearing, and make
the cache helper keep no-store instead of replacing it (pages, raw files
and the event stream).

commit: 41cf2ec8c14a3fd330f49eb8a950be865a601f65
author: brice · 11d ago · 2026-10-10 19:28 UTC
committer: brice · 11d ago · 2026-10-10 19:28 UTC
parent: e2c58e1
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
refs: tag: v0.2.0

5 files · +106 -3

M cmd/ermite/web_session_test.go +4 -2


@@ -266,7 +266,8 @@ func TestWebLoginEndToEnd(t *testing.T) {
266	266		c.cli(t, adminToken, "web", "logout", "--all").ok(t)
267	267		after := b.get("/secret", as(token))
268	268		assert.Equal(t, 404, after.status)
269		-	assert.Equal(t, "no-cache", after.hdr.Get("Cache-Control"))
	269	+	assert.Equal(t, "no-store", after.hdr.Get("Cache-Control"), "a rejected cookie makes the answer uncacheable")
	270	+	assert.Contains(t, after.hdr.Values("Vary"), "Cookie")
270	271		cleared := after.cookie(sessionCookie)
271	272		require.NotNil(t, cleared, "a rejected cookie is cleared")
272	273		assert.Negative(t, cleared.MaxAge)

@@ -376,7 +377,8 @@ func TestWebRejectsForgedSessions(t *testing.T) {
376	377		} {
377	378		p := b.get("/secret", as(tok))
378	379		assert.Equal(t, 404, p.status, name)
379		-	assert.Equal(t, "no-cache", p.hdr.Get("Cache-Control"), name)
	380	+	assert.Equal(t, "no-store", p.hdr.Get("Cache-Control"), name)
	381	+	assert.Contains(t, p.hdr.Values("Vary"), "Cookie", name)
380	382		ck := p.cookie(sessionCookie)
381	383		require.NotNil(t, ck, name)
382	384		assert.Negative(t, ck.MaxAge, name)

 	c.cli(t, adminToken, "web", "logout", "--all").ok(t)
 	after := b.get("/secret", as(token))
 	assert.Equal(t, 404, after.status)
-	assert.Equal(t, "no-cache", after.hdr.Get("Cache-Control"))
+	assert.Equal(t, "no-store", after.hdr.Get("Cache-Control"), "a rejected cookie makes the answer uncacheable")
+	assert.Contains(t, after.hdr.Values("Vary"), "Cookie")
 	cleared := after.cookie(sessionCookie)
 	require.NotNil(t, cleared, "a rejected cookie is cleared")
 	assert.Negative(t, cleared.MaxAge)
 	} {
 		p := b.get("/secret", as(tok))
 		assert.Equal(t, 404, p.status, name)
-		assert.Equal(t, "no-cache", p.hdr.Get("Cache-Control"), name)
+		assert.Equal(t, "no-store", p.hdr.Get("Cache-Control"), name)
+		assert.Contains(t, p.hdr.Values("Vary"), "Cookie", name)
 		ck := p.cookie(sessionCookie)
 		require.NotNil(t, ck, name)
 		assert.Negative(t, ck.MaxAge, name)

M internal/adapters/web/events.go +3 -0


@@ -168,6 +168,9 @@ func (s Server) events(w http.ResponseWriter, r http.Request) {
168	168		hdr := w.Header()
169	169		hdr.Set("Content-Type", "text/event-stream")
170	170		cacheHeaders(ctx, hdr, "no-cache, no-transform", sessionCache+", no-transform")
	171	+	if hdr.Get("Cache-Control") == noStore { // an invalid cookie was cleared
	172	+	hdr.Set("Cache-Control", noStore+", no-transform")
	173	+	}
171	174		hdr.Set("X-Accel-Buffering", "no")
172	175		_ = rc.SetWriteDeadline(time.Now().Add(writeWindow))
173	176		w.WriteHeader(http.StatusOK)


@@ -168,6 +168,9 @@ func (s Server) events(w http.ResponseWriter, r http.Request) {
168	hdr := w.Header()	168		hdr := w.Header()
169	hdr.Set("Content-Type", "text/event-stream")	169		hdr.Set("Content-Type", "text/event-stream")
170	cacheHeaders(ctx, hdr, "no-cache, no-transform", sessionCache+", no-transform")	170		cacheHeaders(ctx, hdr, "no-cache, no-transform", sessionCache+", no-transform")
		171	+	if hdr.Get("Cache-Control") == noStore { // an invalid cookie was cleared
		172	+	hdr.Set("Cache-Control", noStore+", no-transform")
		173	+	}
171	hdr.Set("X-Accel-Buffering", "no")	174		hdr.Set("X-Accel-Buffering", "no")
172	_ = rc.SetWriteDeadline(time.Now().Add(writeWindow))	175		_ = rc.SetWriteDeadline(time.Now().Add(writeWindow))
173	w.WriteHeader(http.StatusOK)	176		w.WriteHeader(http.StatusOK)

M internal/adapters/web/events_test.go +12 -0


@@ -357,3 +357,15 @@ func TestEventsVisitorStreamIgnoresSessions(t *testing.T) {
357	357		}
358	358		assert.True(t, found)
359	359		}
	360	+
	361	+	// GO-006: a stream opened with an invalid cookie is anonymous, and its headers
	362	+	// say the answer must not be stored and varies on the cookie.
	363	+	func TestEventsInvalidCookieStream(t *testing.T) {
	364	+	f := &fakeEvents{release: make(chan struct{})}
	365	+	ts := eventsServerWith(t, f, time.Hour, 0, &fakeSessions{valid: "tok"})
	366	+	resp, _ := open(t, ts, "topic=home", map[string]string{"Cookie": CookieName + "=bad"})
	367	+	require.Equal(t, http.StatusOK, resp.StatusCode)
	368	+	assert.Equal(t, "no-store, no-transform", resp.Header.Get("Cache-Control"))
	369	+	assert.Contains(t, resp.Header.Values("Vary"), "Cookie")
	370	+	assert.Contains(t, resp.Header.Get("Set-Cookie"), "Max-Age=0")
	371	+	}


@@ -357,3 +357,15 @@ func TestEventsVisitorStreamIgnoresSessions(t *testing.T) {
357	}	357		}
358	assert.True(t, found)	358		assert.True(t, found)
359	}	359		}
		360	+
		361	+	// GO-006: a stream opened with an invalid cookie is anonymous, and its headers
		362	+	// say the answer must not be stored and varies on the cookie.
		363	+	func TestEventsInvalidCookieStream(t *testing.T) {
		364	+	f := &fakeEvents{release: make(chan struct{})}
		365	+	ts := eventsServerWith(t, f, time.Hour, 0, &fakeSessions{valid: "tok"})
		366	+	resp, _ := open(t, ts, "topic=home", map[string]string{"Cookie": CookieName + "=bad"})
		367	+	require.Equal(t, http.StatusOK, resp.StatusCode)
		368	+	assert.Equal(t, "no-store, no-transform", resp.Header.Get("Cache-Control"))
		369	+	assert.Contains(t, resp.Header.Values("Vary"), "Cookie")
		370	+	assert.Contains(t, resp.Header.Get("Set-Cookie"), "Max-Age=0")
		371	+	}

M internal/adapters/web/session.go +28 -1


@@ -80,15 +80,37 @@ const sessionCache = "private, no-store"
80	80
81	81		// cacheHeaders sets Cache-Control: public for a visitor, and for the owner
82	82		// the private variant with Vary: Cookie.
	83	+	//
	84	+	// A response that already carries no-store is kept as it is: withSession sets
	85	+	// it when it clears an invalid cookie, and a later public Cache-Control must
	86	+	// not undo it.
83	87		func cacheHeaders(ctx context.Context, h http.Header, public, private string) {
84	88		if SignedIn(ctx) {
85	89		h.Set("Cache-Control", private)
86		-	h.Add("Vary", "Cookie")
	90	+	varyCookie(h)
	91	+	return
	92	+	}
	93	+	if h.Get("Cache-Control") == noStore {
87	94		return
88	95		}
89	96		h.Set("Cache-Control", public)
90	97		}
91	98
	99	+	// noStore is the Cache-Control of an answer no cache may keep.
	100	+	const noStore = "no-store"
	101	+
	102	+	// varyCookie adds Vary: Cookie unless the header already says so.
	103	+	func varyCookie(h http.Header) {
	104	+	for _, v := range h.Values("Vary") {
	105	+	for f := range strings.SplitSeq(v, ",") {
	106	+	if t := strings.TrimSpace(f); t == "*" \|\| strings.EqualFold(t, "Cookie") {
	107	+	return
	108	+	}
	109	+	}
	110	+	}
	111	+	h.Add("Vary", "Cookie")
	112	+	}
	113	+
92	114		// logPath is the path of a request as the log may show it: a login link is
93	115		// a secret. The decoded path is given, so the check is wide on purpose: a
94	116		// path that only reaches the link after cleaning ("/x/../-/login/…",

@@ -120,7 +142,12 @@ func (s Server) withSession(w http.ResponseWriter, r http.Request) *http.Reque
120	142		log.Error().Err(err).Msg("check web session")
121	143		return r
122	144		case !ok:
	145	+	// The answer depends on a cookie that is gone now: no cache keeps it
	146	+	// for another visitor, and none serves it for a request with a cookie.
123	147		http.SetCookie(w, s.cookie("", -1))
	148	+	h := w.Header()
	149	+	h.Set("Cache-Control", noStore)
	150	+	varyCookie(h)
124	151		return r
125	152		}
126	153		return r.WithContext(context.WithValue(owner, signedInKey{}, struct{}{}))


@@ -80,15 +80,37 @@ const sessionCache = "private, no-store"
80			80
81		// cacheHeaders sets Cache-Control: public for a visitor, and for the owner	81		// cacheHeaders sets Cache-Control: public for a visitor, and for the owner
82		// the private variant with Vary: Cookie.	82		// the private variant with Vary: Cookie.
			83	+	//
			84	+	// A response that already carries no-store is kept as it is: withSession sets
			85	+	// it when it clears an invalid cookie, and a later public Cache-Control must
			86	+	// not undo it.
83		func cacheHeaders(ctx context.Context, h http.Header, public, private string) {	87		func cacheHeaders(ctx context.Context, h http.Header, public, private string) {
84		if SignedIn(ctx) {	88		if SignedIn(ctx) {
85		h.Set("Cache-Control", private)	89		h.Set("Cache-Control", private)
86	-	h.Add("Vary", "Cookie")	90	+	varyCookie(h)
			91	+	return
			92	+	}
			93	+	if h.Get("Cache-Control") == noStore {
87		return	94		return
88		}	95		}
89		h.Set("Cache-Control", public)	96		h.Set("Cache-Control", public)
90		}	97		}
91			98
			99	+	// noStore is the Cache-Control of an answer no cache may keep.
			100	+	const noStore = "no-store"
			101	+
			102	+	// varyCookie adds Vary: Cookie unless the header already says so.
			103	+	func varyCookie(h http.Header) {
			104	+	for _, v := range h.Values("Vary") {
			105	+	for f := range strings.SplitSeq(v, ",") {
			106	+	if t := strings.TrimSpace(f); t == "*" \|\| strings.EqualFold(t, "Cookie") {
			107	+	return
			108	+	}
			109	+	}
			110	+	}
			111	+	h.Add("Vary", "Cookie")
			112	+	}
			113	+
92		// logPath is the path of a request as the log may show it: a login link is	114		// logPath is the path of a request as the log may show it: a login link is
93		// a secret. The decoded path is given, so the check is wide on purpose: a	115		// a secret. The decoded path is given, so the check is wide on purpose: a
94		// path that only reaches the link after cleaning ("/x/../-/login/…",	116		// path that only reaches the link after cleaning ("/x/../-/login/…",

@@ -120,7 +142,12 @@ func (s Server) withSession(w http.ResponseWriter, r http.Request) *http.Reque
120		log.Error().Err(err).Msg("check web session")	142		log.Error().Err(err).Msg("check web session")
121		return r	143		return r
122		case !ok:	144		case !ok:
			145	+	// The answer depends on a cookie that is gone now: no cache keeps it
			146	+	// for another visitor, and none serves it for a request with a cookie.
123		http.SetCookie(w, s.cookie("", -1))	147		http.SetCookie(w, s.cookie("", -1))
			148	+	h := w.Header()
			149	+	h.Set("Cache-Control", noStore)
			150	+	varyCookie(h)
124		return r	151		return r
125		}	152		}
126		return r.WithContext(context.WithValue(owner, signedInKey{}, struct{}{}))	153		return r.WithContext(context.WithValue(owner, signedInKey{}, struct{}{}))

M internal/adapters/web/session_test.go +59 -0


@@ -190,6 +190,65 @@ func TestWithSessionFailsClosed(t *testing.T) {
190	190		assert.True(t, SignedIn(srv.withSession(httptest.NewRecorder(), req).Context()))
191	191		}
192	192
	193	+	// GO-006: the answer to a request with an invalid cookie is not cached and
	194	+	// varies on Cookie, and nothing later in the request undoes it: the page
	195	+	// writer, the raw file handler and the event stream keep it.
	196	+	func TestInvalidCookieAnswerIsNotCached(t *testing.T) {
	197	+	sess := &fakeSessions{valid: "tok"}
	198	+	srv, _ := newSessionServer(t, sess, "https://forge.example")
	199	+
	200	+	// withSession alone.
	201	+	req := httptest.NewRequest(http.MethodGet, "/", nil)
	202	+	req.AddCookie(&http.Cookie{Name: CookieName, Value: "bad"})
	203	+	rec := httptest.NewRecorder()
	204	+	srv.withSession(rec, req)
	205	+	assert.Equal(t, "no-store", rec.Header().Get("Cache-Control"))
	206	+	assert.Equal(t, []string{"Cookie"}, rec.Header().Values("Vary"))
	207	+	assert.Contains(t, rec.Header().Get("Set-Cookie"), "Max-Age=0")
	208	+
	209	+	// A check that cannot be made keeps the cookie and the headers as they are.
	210	+	broken, _ := newSessionServer(t, &fakeSessions{valid: "tok", err: errors.New("kv down")}, "https://forge.example")
	211	+	rec = httptest.NewRecorder()
	212	+	broken.withSession(rec, req)
	213	+	assert.Empty(t, rec.Header().Values("Cache-Control"))
	214	+	assert.Empty(t, rec.Header().Values("Vary"))
	215	+
	216	+	// No cookie, a valid cookie: untouched by withSession.
	217	+	rec = httptest.NewRecorder()
	218	+	srv.withSession(rec, httptest.NewRequest(http.MethodGet, "/", nil))
	219	+	assert.Empty(t, rec.Header().Values("Cache-Control"))
	220	+	assert.Empty(t, rec.Header().Values("Vary"))
	221	+
	222	+	// The page writer, a 404 and a 500 included, does not replace it.
	223	+	for _, target := range []string{"/", "/nope/refs", "/x/tree/main"} {
	224	+	req := httptest.NewRequest(http.MethodGet, target, nil)
	225	+	req.AddCookie(&http.Cookie{Name: CookieName, Value: "bad"})
	226	+	rec := httptest.NewRecorder()
	227	+	srv.ServeHTTP(rec, req)
	228	+	assert.Equal(t, "no-store", rec.Header().Get("Cache-Control"), target)
	229	+	assert.Equal(t, []string{"Cookie"}, rec.Header().Values("Vary"), target)
	230	+	assert.Contains(t, rec.Header().Get("Set-Cookie"), "Max-Age=0", target)
	231	+	}
	232	+
	233	+	// The shared helper keeps no-store for a visitor, and replaces it for the
	234	+	// owner (whose answer is private anyway).
	235	+	h := http.Header{"Cache-Control": {"no-store"}, "Vary": {"Accept, cookie"}}
	236	+	cacheHeaders(context.Background(), h, "no-cache", sessionCache)
	237	+	assert.Equal(t, []string{"no-store"}, h.Values("Cache-Control"))
	238	+	assert.Equal(t, []string{"Accept, cookie"}, h.Values("Vary"), "no second Vary: Cookie")
	239	+	owner := context.WithValue(context.Background(), signedInKey{}, struct{}{})
	240	+	cacheHeaders(owner, h, "no-cache", sessionCache)
	241	+	assert.Equal(t, []string{sessionCache}, h.Values("Cache-Control"))
	242	+
	243	+	// Static assets are not touched by sessions at all, cookie or not.
	244	+	req = httptest.NewRequest(http.MethodGet, StaticPrefix+"nope", nil)
	245	+	req.AddCookie(&http.Cookie{Name: CookieName, Value: "bad"})
	246	+	rec = httptest.NewRecorder()
	247	+	srv.ServeHTTP(rec, req)
	248	+	assert.Empty(t, rec.Header().Values("Set-Cookie"))
	249	+	assert.Empty(t, rec.Header().Values("Vary"))
	250	+	}
	251	+
193	252		// Without sessions configured there is no login route and no cookie is read.
194	253		func TestNoSessionsNoRoutes(t *testing.T) {
195	254		srv, err := New(Config{


@@ -190,6 +190,65 @@ func TestWithSessionFailsClosed(t *testing.T) {
190	assert.True(t, SignedIn(srv.withSession(httptest.NewRecorder(), req).Context()))	190		assert.True(t, SignedIn(srv.withSession(httptest.NewRecorder(), req).Context()))
191	}	191		}
192		192
		193	+	// GO-006: the answer to a request with an invalid cookie is not cached and
		194	+	// varies on Cookie, and nothing later in the request undoes it: the page
		195	+	// writer, the raw file handler and the event stream keep it.
		196	+	func TestInvalidCookieAnswerIsNotCached(t *testing.T) {
		197	+	sess := &fakeSessions{valid: "tok"}
		198	+	srv, _ := newSessionServer(t, sess, "https://forge.example")
		199	+
		200	+	// withSession alone.
		201	+	req := httptest.NewRequest(http.MethodGet, "/", nil)
		202	+	req.AddCookie(&http.Cookie{Name: CookieName, Value: "bad"})
		203	+	rec := httptest.NewRecorder()
		204	+	srv.withSession(rec, req)
		205	+	assert.Equal(t, "no-store", rec.Header().Get("Cache-Control"))
		206	+	assert.Equal(t, []string{"Cookie"}, rec.Header().Values("Vary"))
		207	+	assert.Contains(t, rec.Header().Get("Set-Cookie"), "Max-Age=0")
		208	+
		209	+	// A check that cannot be made keeps the cookie and the headers as they are.
		210	+	broken, _ := newSessionServer(t, &fakeSessions{valid: "tok", err: errors.New("kv down")}, "https://forge.example")
		211	+	rec = httptest.NewRecorder()
		212	+	broken.withSession(rec, req)
		213	+	assert.Empty(t, rec.Header().Values("Cache-Control"))
		214	+	assert.Empty(t, rec.Header().Values("Vary"))
		215	+
		216	+	// No cookie, a valid cookie: untouched by withSession.
		217	+	rec = httptest.NewRecorder()
		218	+	srv.withSession(rec, httptest.NewRequest(http.MethodGet, "/", nil))
		219	+	assert.Empty(t, rec.Header().Values("Cache-Control"))
		220	+	assert.Empty(t, rec.Header().Values("Vary"))
		221	+
		222	+	// The page writer, a 404 and a 500 included, does not replace it.
		223	+	for _, target := range []string{"/", "/nope/refs", "/x/tree/main"} {
		224	+	req := httptest.NewRequest(http.MethodGet, target, nil)
		225	+	req.AddCookie(&http.Cookie{Name: CookieName, Value: "bad"})
		226	+	rec := httptest.NewRecorder()
		227	+	srv.ServeHTTP(rec, req)
		228	+	assert.Equal(t, "no-store", rec.Header().Get("Cache-Control"), target)
		229	+	assert.Equal(t, []string{"Cookie"}, rec.Header().Values("Vary"), target)
		230	+	assert.Contains(t, rec.Header().Get("Set-Cookie"), "Max-Age=0", target)
		231	+	}
		232	+
		233	+	// The shared helper keeps no-store for a visitor, and replaces it for the
		234	+	// owner (whose answer is private anyway).
		235	+	h := http.Header{"Cache-Control": {"no-store"}, "Vary": {"Accept, cookie"}}
		236	+	cacheHeaders(context.Background(), h, "no-cache", sessionCache)
		237	+	assert.Equal(t, []string{"no-store"}, h.Values("Cache-Control"))
		238	+	assert.Equal(t, []string{"Accept, cookie"}, h.Values("Vary"), "no second Vary: Cookie")
		239	+	owner := context.WithValue(context.Background(), signedInKey{}, struct{}{})
		240	+	cacheHeaders(owner, h, "no-cache", sessionCache)
		241	+	assert.Equal(t, []string{sessionCache}, h.Values("Cache-Control"))
		242	+
		243	+	// Static assets are not touched by sessions at all, cookie or not.
		244	+	req = httptest.NewRequest(http.MethodGet, StaticPrefix+"nope", nil)
		245	+	req.AddCookie(&http.Cookie{Name: CookieName, Value: "bad"})
		246	+	rec = httptest.NewRecorder()
		247	+	srv.ServeHTTP(rec, req)
		248	+	assert.Empty(t, rec.Header().Values("Set-Cookie"))
		249	+	assert.Empty(t, rec.Header().Values("Vary"))
		250	+	}
		251	+
193	// Without sessions configured there is no login route and no cookie is read.	252		// Without sessions configured there is no login route and no cookie is read.
194	func TestNoSessionsNoRoutes(t *testing.T) {	253		func TestNoSessionsNoRoutes(t *testing.T) {
195	srv, err := New(Config{	254		srv, err := New(Config{