fix: resolve four Semgrep security findings (#48)

Default admin cookies to Secure except for explicit local HTTP.
Direct TLS always enables Secure. OAuth state permits Strict cookies.

Scope suppressions to safe parser IDs and JSON-encoded NDJSON.
Test cookie modes, export headers, and receipt text encoding.

Signed-off-by: Shreyansh Sancheti <43677304+shreyanshjain7174@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: shreyanshjain7174

README.md

@@ -369,7 +369,7 @@ Set `AGENTGATE_VAULT_CACHE_TTL=0` to disable plaintext caching. Refresh sharing
 | `AGENTGATE_VAULT_KEY` | 32-byte vault key. Also derives receipt encryption keys and the API-key HMAC pepper. | Yes |
 | `AGENTGATE_VAULT_CACHE_TTL` | Decrypted-token cache duration. Default `60s`; `0` disables caching. Negative or invalid durations stop startup. | No |
 | `AGENTGATE_ADMIN_SECRET` | Secret for admin API access | Yes |
-| `AGENTGATE_PUBLIC_URL` | Base URL used to build the OAuth callback (default `http://localhost:8080`) | No |
+| `AGENTGATE_PUBLIC_URL` | Public base URL for OAuth callbacks and admin cookie security. Default `http://localhost:8080`. | No |
 | `AGENTGATE_API_KEY_CACHE_TTL` | Verified-key cache duration. Default `30s`; `0` disables caching. | No |
 | `AGENTGATE_RATE_KEY_RPS` | Requests per second per API key ID. Default `20`. Zero requires tenant admission disabled. | No |
 | `AGENTGATE_RATE_KEY_BURST` | Per-key token bucket burst. Default `40`. Zero requires tenant admission disabled. | No |
@@ -453,6 +453,16 @@ The early rate limiter runs before these checks. Structural caps then bound fact
 Biscuit-go v2.2.0 deadlines do not cover check evaluation and cannot reliably stop timed-out goroutines.
 Runtime limits alone do not bound per-request CPU.
 
+### Admin Session Cookies
+
+Set `AGENTGATE_PUBLIC_URL` to your HTTPS address behind a TLS-terminating proxy.
+Admin cookies default to `Secure`, `HttpOnly`, and `SameSite=Strict`, with a 1-hour lifetime and `Path=/`.
+Only explicit HTTP URLs on `localhost` or `127.0.0.1` disable `Secure` for local development.
+Direct TLS requests always set `Secure`, including when the public URL uses local HTTP.
+Forwarded headers cannot downgrade cookie security.
+Logout clears the cookie with the same security attributes.
+OAuth callbacks validate encrypted state without an admin session, so they don't need `SameSite=Lax`.
+
 ### Service Configs
 
 Service configurations are YAML files in `configs/services/`. See `configs/services/google_workspace.yaml` for an example. The gateway itself loads the merged `configs/services.yaml`.

cmd/agentgate-mcp/main.go

@@ -59,6 +59,8 @@ func serve(ctx context.Context, input io.Reader, output io.Writer, client *sdk.C
 	scanner.Buffer(make([]byte, 4096), maxLineBytes)
 	encoder := json.NewEncoder(output)
 	for scanner.Scan() {
+		// parseRequest returns a value with an echoable ID on errors, never a nil pointer.
+		// nosemgrep: trailofbits.go.invalid-usage-of-modified-variable.invalid-usage-of-modified-variable
 		req, envelopeError := parseRequest(scanner.Bytes())
 		if envelopeError != nil {
 			if err := encoder.Encode(map[string]any{"jsonrpc": "2.0", "id": req.ID, "error": envelopeError}); err != nil {

internal/admin/handler.go

@@ -118,12 +118,15 @@ func (h *Handler) Login(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "login failed", http.StatusInternalServerError)
 		return
 	}
+	cookie.Secure = cookie.Secure || r.TLS != nil
 	http.SetCookie(w, cookie)
 	http.Redirect(w, r, "/admin/login", http.StatusSeeOther)
 }
 
 func (h *Handler) Logout(w http.ResponseWriter, r *http.Request) {
-	http.SetCookie(w, h.sessions.ClearCookie())
+	cookie := h.sessions.ClearCookie()
+	cookie.Secure = cookie.Secure || r.TLS != nil
+	http.SetCookie(w, cookie)
 	http.Redirect(w, r, "/admin/login", http.StatusSeeOther)
 }
 

internal/admin/handler_test.go

@@ -65,6 +65,68 @@ func TestHandlerLoginSetsSessionCookie(t *testing.T) {
 	}
 }
 
+func TestHandlerSessionCookieSecurity(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name           string
+		publicURL      string
+		requestURL     string
+		forwardedProto string
+		wantSecure     bool
+	}{
+		{name: "https proxy", publicURL: "https://agentgate.example", requestURL: "http://agentgate.example", wantSecure: true},
+		{name: "localhost development", publicURL: "http://localhost:8080", requestURL: "http://localhost:8080"},
+		{name: "loopback development", publicURL: "http://127.0.0.1:8080", requestURL: "http://127.0.0.1:8080"},
+		{name: "direct tls", publicURL: "http://localhost:8080", requestURL: "https://localhost:8080", wantSecure: true},
+		{name: "remote http", publicURL: "http://agentgate.example", requestURL: "http://agentgate.example", wantSecure: true},
+		{name: "missing public url", requestURL: "http://agentgate.example", wantSecure: true},
+		{name: "forwarded http cannot downgrade", publicURL: "https://agentgate.example", requestURL: "http://agentgate.example", forwardedProto: "http", wantSecure: true},
+		{name: "forwarded https is not trusted", publicURL: "http://localhost:8080", requestURL: "http://localhost:8080", forwardedProto: "https"},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			handler, _ := testHandler(t)
+			handler.sessions = NewSessionManager([]byte("dev-key-change-in-production-32b"), test.publicURL)
+			form := url.Values{"email": {"admin@example.com"}, "password": {"correct horse battery staple"}}
+			request := httptest.NewRequest(http.MethodPost, test.requestURL+"/admin/login", bytes.NewBufferString(form.Encode()))
+			request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+			request.Header.Set("X-Forwarded-Proto", test.forwardedProto)
+			response := httptest.NewRecorder()
+			handler.Login(response, request)
+			if response.Code != http.StatusSeeOther {
+				t.Fatalf("Login status = %d, want %d", response.Code, http.StatusSeeOther)
+			}
+			loginCookies := response.Result().Cookies()
+			if len(loginCookies) != 1 {
+				t.Fatalf("Login returned %d cookies, want 1", len(loginCookies))
+			}
+
+			logoutRequest := httptest.NewRequest(http.MethodPost, test.requestURL+"/admin/logout", nil)
+			logoutRequest.Header.Set("X-Forwarded-Proto", test.forwardedProto)
+			logoutRequest.Header.Set(csrfHeaderName, csrfHeaderValue)
+			logoutRequest.AddCookie(loginCookies[0])
+			logoutResponse := httptest.NewRecorder()
+			handler.RequireAdmin(http.HandlerFunc(handler.Logout)).ServeHTTP(logoutResponse, logoutRequest)
+			if logoutResponse.Code != http.StatusSeeOther {
+				t.Fatalf("Logout status = %d, want %d", logoutResponse.Code, http.StatusSeeOther)
+			}
+			logoutCookies := logoutResponse.Result().Cookies()
+			if len(logoutCookies) != 1 || logoutCookies[0].MaxAge != -1 {
+				t.Fatalf("Logout cookies = %#v", logoutCookies)
+			}
+			for _, cookie := range []*http.Cookie{loginCookies[0], logoutCookies[0]} {
+				if cookie.Secure != test.wantSecure || !cookie.HttpOnly ||
+					cookie.SameSite != http.SameSiteStrictMode || cookie.Path != "/" {
+					t.Fatalf("cookie has incorrect attributes: %#v", cookie)
+				}
+			}
+			if loginCookies[0].MaxAge != int(sessionTTL.Seconds()) || loginCookies[0].Expires.IsZero() {
+				t.Fatalf("Login cookie has incorrect lifetime: %#v", loginCookies[0])
+			}
+		})
+	}
+}
+
 func TestHandlerRequireAdmin(t *testing.T) {
 	t.Parallel()
 	handler, identity := testHandler(t)

internal/admin/session.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"strings"
 	"time"
 
@@ -42,9 +43,15 @@ type SessionManager struct {
 }
 
 func NewSessionManager(masterKey []byte, publicURL string) *SessionManager {
+	secure := true
+	parsed, err := url.Parse(publicURL)
+	if err == nil && parsed.Scheme == "http" && parsed.User == nil &&
+		(strings.EqualFold(parsed.Hostname(), "localhost") || parsed.Hostname() == "127.0.0.1") {
+		secure = false
+	}
 	return &SessionManager{
 		key:    signer.DerivePurposeKey(masterKey, sessionKeyPurpose),
-		secure: strings.HasPrefix(publicURL, "https://"),
+		secure: secure,
 		now:    time.Now,
 	}
 }
@@ -76,6 +83,8 @@ func (m *SessionManager) CreateCookie(identity SessionIdentity) (*http.Cookie, e
 	}
 	ciphertext := gcm.Seal(nonce, nonce, payload, nil)
 	expiresAt := m.now().Add(sessionTTL)
+	// Secure is disabled only for explicit loopback HTTP development URLs.
+	// nosemgrep: go.lang.security.audit.net.cookie-missing-secure.cookie-missing-secure
 	return &http.Cookie{
 		Name:     sessionCookieName,
 		Value:    base64.URLEncoding.EncodeToString(ciphertext),
@@ -84,7 +93,7 @@ func (m *SessionManager) CreateCookie(identity SessionIdentity) (*http.Cookie, e
 		MaxAge:   int(sessionTTL.Seconds()),
 		HttpOnly: true,
 		Secure:   m.secure,
-		SameSite: http.SameSiteLaxMode,
+		SameSite: http.SameSiteStrictMode,
 	}, nil
 }
 
@@ -125,13 +134,15 @@ func (m *SessionManager) Authenticate(r *http.Request) (SessionIdentity, error)
 }
 
 func (m *SessionManager) ClearCookie() *http.Cookie {
+	// Secure is disabled only for explicit loopback HTTP development URLs.
+	// nosemgrep: go.lang.security.audit.net.cookie-missing-secure.cookie-missing-secure
 	return &http.Cookie{
 		Name:     sessionCookieName,
 		Value:    "",
 		Path:     "/",
 		MaxAge:   -1,
 		HttpOnly: true,
 		Secure:   m.secure,
-		SameSite: http.SameSiteLaxMode,
+		SameSite: http.SameSiteStrictMode,
 	}
 }

internal/admin/session_test.go

@@ -26,7 +26,7 @@ func TestSessionManagerAuthenticate(t *testing.T) {
 	if identity != (SessionIdentity{AdminID: "admin-1", OrgID: "org-1"}) {
 		t.Fatalf("identity = %#v", identity)
 	}
-	if !cookie.HttpOnly || !cookie.Secure || cookie.SameSite != http.SameSiteLaxMode || cookie.Path != "/" {
+	if !cookie.HttpOnly || !cookie.Secure || cookie.SameSite != http.SameSiteStrictMode || cookie.Path != "/" {
 		t.Fatalf("cookie has unsafe attributes: %#v", cookie)
 	}
 }
@@ -71,16 +71,47 @@ func TestSessionManagerRejectsInvalidSessions(t *testing.T) {
 
 func TestSessionManagerCookieSecurityFollowsPublicURL(t *testing.T) {
 	t.Parallel()
-	manager := NewSessionManager([]byte("dev-key-change-in-production-32b"), "http://localhost:8080")
-	cookie, err := manager.CreateCookie(SessionIdentity{AdminID: "admin-1", OrgID: "org-1"})
-	if err != nil {
-		t.Fatalf("CreateCookie: %v", err)
-	}
-	if cookie.Secure {
-		t.Fatal("http public URL set Secure cookie")
+	tests := []struct {
+		name       string
+		publicURL  string
+		wantSecure bool
+	}{
+		{name: "https", publicURL: "https://agentgate.example", wantSecure: true},
+		{name: "uppercase https", publicURL: "HTTPS://agentgate.example", wantSecure: true},
+		{name: "https localhost", publicURL: "https://localhost:8080", wantSecure: true},
+		{name: "http localhost", publicURL: "http://localhost:8080"},
+		{name: "http uppercase localhost", publicURL: "http://LOCALHOST:8080"},
+		{name: "http loopback", publicURL: "http://127.0.0.1:8080"},
+		{name: "http remote", publicURL: "http://agentgate.example", wantSecure: true},
+		{name: "http lookalike", publicURL: "http://localhost.example", wantSecure: true},
+		{name: "http userinfo", publicURL: "http://remote@localhost", wantSecure: true},
+		{name: "http ipv6", publicURL: "http://[::1]:8080", wantSecure: true},
+		{name: "missing url", wantSecure: true},
+		{name: "relative url", publicURL: "/localhost", wantSecure: true},
+		{name: "malformed url", publicURL: "http://localhost:%", wantSecure: true},
 	}
-	cleared := manager.ClearCookie()
-	if cleared.MaxAge != -1 || !cleared.HttpOnly || cleared.SameSite != http.SameSiteLaxMode {
-		t.Fatalf("ClearCookie = %#v", cleared)
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			now := time.Date(2026, time.September, 1, 12, 0, 0, 0, time.UTC)
+			manager := NewSessionManager([]byte("dev-key-change-in-production-32b"), test.publicURL)
+			manager.now = func() time.Time { return now }
+			cookie, err := manager.CreateCookie(SessionIdentity{AdminID: "admin-1", OrgID: "org-1"})
+			if err != nil {
+				t.Fatalf("CreateCookie: %v", err)
+			}
+			if cookie.Name != sessionCookieName || cookie.Value == "" ||
+				cookie.MaxAge != int(sessionTTL.Seconds()) || !cookie.Expires.Equal(now.Add(sessionTTL)) {
+				t.Fatalf("CreateCookie = %#v", cookie)
+			}
+			cleared := manager.ClearCookie()
+			if cleared.Name != sessionCookieName || cleared.Value != "" || cleared.MaxAge != -1 {
+				t.Fatalf("ClearCookie = %#v", cleared)
+			}
+			for _, c := range []*http.Cookie{cookie, cleared} {
+				if c.Secure != test.wantSecure || !c.HttpOnly || c.SameSite != http.SameSiteStrictMode || c.Path != "/" {
+					t.Fatalf("cookie has incorrect attributes: %#v", c)
+				}
+			}
+		})
 	}
 }

internal/receipt/export_handler.go

@@ -227,6 +227,8 @@ func writeJSONLLine(w http.ResponseWriter, line []byte, err error) {
 	if err != nil {
 		return // the response is already committed by the time marshaling could fail; nothing safe left to do
 	}
+	// Callers supply JSON-encoded records under application/x-ndjson with nosniff, not HTML.
+	// nosemgrep: go.lang.security.audit.xss.no-direct-write-to-responsewriter.no-direct-write-to-responsewriter
 	w.Write(line)
 	w.Write([]byte("\n"))
 }

internal/receipt/export_handler_test.go

@@ -24,9 +24,12 @@ func TestExportHandler_FullRangeVerifiesOffline(t *testing.T) {
 	if w.Code != http.StatusOK {
 		t.Fatalf("status = %d, body = %s", w.Code, w.Body.String())
 	}
-	if ct := w.Header().Get("Content-Type"); ct != "application/x-ndjson" {
+	if ct := w.Result().Header.Get("Content-Type"); ct != "application/x-ndjson" {
 		t.Fatalf("Content-Type = %s, want application/x-ndjson", ct)
 	}
+	if got := w.Result().Header.Get("X-Content-Type-Options"); got != "nosniff" {
+		t.Fatalf("X-Content-Type-Options = %q, want nosniff", got)
+	}
 
 	receipts, keys, manifest := parseExportBody(t, w.Body.String())
 	if manifest == nil {
@@ -122,6 +125,12 @@ func TestExportHandler_InvalidRangesReject400(t *testing.T) {
 		if w.Code != http.StatusBadRequest {
 			t.Errorf("%s: status = %d, want 400", url, w.Code)
 		}
+		if ct := w.Result().Header.Get("Content-Type"); ct != "application/json" {
+			t.Errorf("%s: Content-Type = %q, want application/json", url, ct)
+		}
+		if got := w.Result().Header.Get("X-Content-Type-Options"); got != "nosniff" {
+			t.Errorf("%s: X-Content-Type-Options = %q, want nosniff", url, got)
+		}
 	}
 }
 
@@ -219,6 +228,29 @@ func TestExportHandler_NoRawParamsOrSensitiveData(t *testing.T) {
 	}
 }
 
+func TestExportHandler_JSONEncodesReceiptText(t *testing.T) {
+	t.Parallel()
+	ledger, store := newTestLedgerAndStore(t)
+	draft := testDraft("github", "list_repos")
+	draft.HumanPrincipal = "</script><script>alert(\"receipt\")</script>\n{\"injected\":true}"
+	if _, err := ledger.Append(t.Context(), draft); err != nil {
+		t.Fatalf("Append: %v", err)
+	}
+	request := httptest.NewRequest(http.MethodGet, "/v1/receipts/export?from=1", nil)
+	response := httptest.NewRecorder()
+	ExportHandler(ledger.db, store)(response, request)
+	if response.Code != http.StatusOK {
+		t.Fatalf("status = %d, body = %s", response.Code, response.Body.String())
+	}
+	if strings.Contains(response.Body.String(), "<script>") {
+		t.Fatal("export contains unescaped HTML")
+	}
+	receipts, _, _ := parseExportBody(t, response.Body.String())
+	if len(receipts) != 1 || receipts[0].HumanPrincipal != draft.HumanPrincipal {
+		t.Fatalf("export changed or injected receipt data: %#v", receipts)
+	}
+}
+
 func parseExportBody(t *testing.T, body string) (receipts []Receipt, keys []TrustedKey, manifest *ExportManifest) {
 	t.Helper()
 	for _, line := range strings.Split(strings.TrimSpace(body), "\n") {