Close remaining CodeQL allocation and cookie gaps

This commit is contained in:
Richard Courtman
2026-07-09 20:10:08 +01:00
parent 042e7ef966
commit 1968dc4171
11 changed files with 96 additions and 43 deletions
+20 -6
View File
@@ -345,13 +345,27 @@ func getBrowserCookiePolicy(r *http.Request) browserCookiePolicy {
}
}
// set writes a browser cookie using the one request-derived policy shared by
// every authentication flow. Plain HTTP remains limited to the supported
// loopback/self-hosted boundary; public deployments receive Secure cookies.
func (p browserCookiePolicy) set(w http.ResponseWriter, cookie *http.Cookie) {
// setClientReadable writes a browser cookie that the frontend must read.
// Plain HTTP remains limited to the supported loopback/self-hosted boundary;
// public deployments receive Secure cookies.
func (p browserCookiePolicy) setClientReadable(w http.ResponseWriter, cookie *http.Cookie) {
if w == nil || cookie == nil {
return
}
cookie.HttpOnly = false
cookie.Secure = p.secure
cookie.SameSite = p.sameSite
http.SetCookie(w, cookie)
}
// setHTTPOnly writes an authentication cookie that must never be available to
// browser scripts. Keeping this as a distinct sink makes the security contract
// independent of each caller's cookie literal.
func (p browserCookiePolicy) setHTTPOnly(w http.ResponseWriter, cookie *http.Cookie) {
if w == nil || cookie == nil {
return
}
cookie.HttpOnly = true
cookie.Secure = p.secure
cookie.SameSite = p.sameSite
http.SetCookie(w, cookie)
@@ -914,7 +928,7 @@ func checkAuth(cfg *config.Config, w http.ResponseWriter, r *http.Request, write
Msg("Setting session cookie after successful login")
// Set session cookie
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: sessionCookieName(cookiePolicy.secure),
Value: token,
Path: "/",
@@ -923,7 +937,7 @@ func checkAuth(cfg *config.Config, w http.ResponseWriter, r *http.Request, write
})
// Set CSRF cookie (not HttpOnly so JS can read it)
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
+24 -5
View File
@@ -335,7 +335,7 @@ func TestGetCookieSettings(t *testing.T) {
}
}
func TestBrowserCookiePolicyAppliesRequestSecurity(t *testing.T) {
func TestBrowserCookiePolicyAppliesRequestSecurityAndHTTPOnlyContract(t *testing.T) {
tests := []struct {
name string
secure bool
@@ -352,11 +352,11 @@ func TestBrowserCookiePolicyAppliesRequestSecurity(t *testing.T) {
req.TLS = &tls.ConnectionState{}
}
rec := httptest.NewRecorder()
getBrowserCookiePolicy(req).set(rec, &http.Cookie{
Name: CookieNameCSRF,
getBrowserCookiePolicy(req).setHTTPOnly(rec, &http.Cookie{
Name: cookieNameSession,
Value: "token",
Path: "/",
HttpOnly: true,
HttpOnly: false,
})
cookies := rec.Result().Cookies()
@@ -371,12 +371,31 @@ func TestBrowserCookiePolicyAppliesRequestSecurity(t *testing.T) {
t.Fatalf("SameSite = %v, want Lax", cookie.SameSite)
}
if !cookie.HttpOnly {
t.Fatal("cookie-specific attributes must be preserved")
t.Fatal("authentication cookie must be forced HttpOnly")
}
})
}
}
func TestBrowserCookiePolicyKeepsClientCookieReadable(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "https://pulse.example/", nil)
rec := httptest.NewRecorder()
getBrowserCookiePolicy(req).setClientReadable(rec, &http.Cookie{
Name: CookieNameCSRF,
Value: "token",
Path: "/",
HttpOnly: true,
})
cookies := rec.Result().Cookies()
if len(cookies) != 1 {
t.Fatalf("cookies = %d, want 1", len(cookies))
}
if cookies[0].HttpOnly {
t.Fatal("client-readable cookie must not be HttpOnly")
}
}
func TestGenerateSessionToken(t *testing.T) {
// Test that tokens are generated
token := generateSessionToken()
+3 -3
View File
@@ -117,20 +117,20 @@ func HandleCloudHandoff(dataPath string) http.HandlerFunc {
cookiePolicy := getBrowserCookiePolicy(r)
cookieMaxAge := int(sessionDuration.Seconds())
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: sessionCookieName(cookiePolicy.secure),
Value: sessionToken,
Path: "/",
HttpOnly: true,
MaxAge: cookieMaxAge,
})
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
MaxAge: cookieMaxAge,
})
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameOrgID,
Value: tenantID,
Path: "/",
+3 -3
View File
@@ -409,20 +409,20 @@ func HandleHandoffExchange(configDir string) http.HandlerFunc {
cookiePolicy := getBrowserCookiePolicy(r)
cookieMaxAge := int(sessionDuration.Seconds())
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: sessionCookieName(cookiePolicy.secure),
Value: sessionToken,
Path: "/",
HttpOnly: true,
MaxAge: cookieMaxAge,
})
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
MaxAge: cookieMaxAge,
})
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameOrgID,
Value: tenantID,
Path: "/",
+3 -3
View File
@@ -195,14 +195,14 @@ func (h *MagicLinkHandlers) HandlePublicMagicLinkVerify(w http.ResponseWriter, r
cookiePolicy := getBrowserCookiePolicy(r)
cookieMaxAge := int(sessionDuration.Seconds())
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: sessionCookieName(cookiePolicy.secure),
Value: sessionToken,
Path: "/",
HttpOnly: true,
MaxAge: cookieMaxAge,
})
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
@@ -210,7 +210,7 @@ func (h *MagicLinkHandlers) HandlePublicMagicLinkVerify(w http.ResponseWriter, r
})
// Org cookie is intentionally NOT HttpOnly — the frontend reads/writes it to
// synchronize org context for WebSocket connections (which cannot send custom headers).
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameOrgID,
Value: token.OrgID,
Path: "/",
+9 -9
View File
@@ -4215,7 +4215,7 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
if csrfErr != nil {
// Session exists but no CSRF cookie - issue one
csrfToken := generateCSRFToken(sessionCookie.Value)
getBrowserCookiePolicy(req).set(w, &http.Cookie{
getBrowserCookiePolicy(req).setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
@@ -4977,7 +4977,7 @@ func (r *Router) establishSession(w http.ResponseWriter, req *http.Request, user
csrfToken := generateCSRFToken(token)
cookiePolicy := getBrowserCookiePolicy(req)
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: sessionCookieName(cookiePolicy.secure),
Value: token,
Path: "/",
@@ -4985,7 +4985,7 @@ func (r *Router) establishSession(w http.ResponseWriter, req *http.Request, user
MaxAge: 86400,
})
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
@@ -5014,7 +5014,7 @@ func (r *Router) establishRecoverySession(w http.ResponseWriter, req *http.Reque
csrfToken := generateCSRFToken(token)
cookiePolicy := getBrowserCookiePolicy(req)
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: sessionCookieName(cookiePolicy.secure),
Value: token,
Path: "/",
@@ -5022,7 +5022,7 @@ func (r *Router) establishRecoverySession(w http.ResponseWriter, req *http.Reque
MaxAge: 86400,
})
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
@@ -5055,7 +5055,7 @@ func (r *Router) establishOIDCSession(w http.ResponseWriter, req *http.Request,
csrfToken := generateCSRFToken(token)
cookiePolicy := getBrowserCookiePolicy(req)
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: sessionCookieName(cookiePolicy.secure),
Value: token,
Path: "/",
@@ -5063,7 +5063,7 @@ func (r *Router) establishOIDCSession(w http.ResponseWriter, req *http.Request,
MaxAge: 86400,
})
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
@@ -5184,7 +5184,7 @@ func (r *Router) handleLogin(w http.ResponseWriter, req *http.Request) {
cookieMaxAge := int(sessionDuration.Seconds())
// Set session cookie
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: sessionCookieName(cookiePolicy.secure),
Value: token,
Path: "/",
@@ -5193,7 +5193,7 @@ func (r *Router) handleLogin(w http.ResponseWriter, req *http.Request) {
})
// Set CSRF cookie (not HttpOnly so JS can read it)
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
+4 -4
View File
@@ -524,7 +524,7 @@ func (r *Router) establishSAMLSession(w http.ResponseWriter, req *http.Request,
csrfToken := generateCSRFToken(token)
cookiePolicy := getBrowserCookiePolicy(req)
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: sessionCookieName(cookiePolicy.secure),
Value: token,
Path: "/",
@@ -532,7 +532,7 @@ func (r *Router) establishSAMLSession(w http.ResponseWriter, req *http.Request,
MaxAge: 86400,
})
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: csrfToken,
Path: "/",
@@ -584,7 +584,7 @@ func (r *Router) clearSession(w http.ResponseWriter, req *http.Request) {
// Clear both session cookie variants (prefixed and unprefixed)
for _, name := range []string{cookieNameSession, cookieNameSessionSecure} {
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setHTTPOnly(w, &http.Cookie{
Name: name,
Value: "",
Path: "/",
@@ -594,7 +594,7 @@ func (r *Router) clearSession(w http.ResponseWriter, req *http.Request) {
}
// Clear pulse_csrf cookie
cookiePolicy.set(w, &http.Cookie{
cookiePolicy.setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: "",
Path: "/",
+2 -2
View File
@@ -132,7 +132,7 @@ func clearCSRFCookie(w http.ResponseWriter, r *http.Request) {
if w == nil {
return
}
getBrowserCookiePolicy(r).set(w, &http.Cookie{
getBrowserCookiePolicy(r).setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: "",
Path: "/",
@@ -150,7 +150,7 @@ func issueNewCSRFCookie(w http.ResponseWriter, r *http.Request, sessionID string
}
newToken := generateCSRFToken(sessionID)
getBrowserCookiePolicy(r).set(w, &http.Cookie{
getBrowserCookiePolicy(r).setClientReadable(w, &http.Cookie{
Name: CookieNameCSRF,
Value: newToken,
Path: "/",
+1 -1
View File
@@ -33,7 +33,7 @@ type wsRawMessage struct {
func TestBrowserCookiePolicyWritesSecureCookiesForTLS(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "https://pulse.example/api/config", nil)
rec := httptest.NewRecorder()
getBrowserCookiePolicy(req).set(rec, &http.Cookie{Name: CookieNameCSRF, Value: "token", Path: "/"})
getBrowserCookiePolicy(req).setClientReadable(rec, &http.Cookie{Name: CookieNameCSRF, Value: "token", Path: "/"})
cookies := rec.Result().Cookies()
if len(cookies) != 1 || !cookies[0].Secure {
+23 -6
View File
@@ -627,13 +627,30 @@ func (c *CommandClient) sendDeployProgress(
}
func makeSemaphore(maxParallel int) chan struct{} {
if maxParallel <= 0 {
maxParallel = 1
// Use literal capacities so network-derived values never reach an allocation
// size, even after normalization.
switch agentexec.NormalizeDeployMaxParallel(maxParallel, 1) {
case 1:
return make(chan struct{}, 1)
case 2:
return make(chan struct{}, 2)
case 3:
return make(chan struct{}, 3)
case 4:
return make(chan struct{}, 4)
case 5:
return make(chan struct{}, 5)
case 6:
return make(chan struct{}, 6)
case 7:
return make(chan struct{}, 7)
case 8:
return make(chan struct{}, 8)
case 9:
return make(chan struct{}, 9)
default:
return make(chan struct{}, agentexec.MaxDeployParallel)
}
if maxParallel > agentexec.MaxDeployParallel {
maxParallel = agentexec.MaxDeployParallel
}
return make(chan struct{}, maxParallel)
}
func marshalPreflightResult(sshOK, pulseReachable, hasAgent bool, arch, errDetail string) string {
+4 -1
View File
@@ -822,7 +822,10 @@ func generateNodes(config MockConfig) []models.Node {
if config.NodeCount > maxMockNodeCount {
config.NodeCount = maxMockNodeCount
}
nodes := make([]models.Node, 0, config.NodeCount)
// Capacity is fixed at the product maximum so request-derived configuration
// never controls an allocation size. The normalized count still controls how
// many nodes are generated.
nodes := make([]models.Node, 0, maxMockNodeCount)
// First 5 nodes are part of the cluster
clusterNodeCount := 5