stelloauth/patches/stelloauth-security.patch

@@ -1,5 +1,5 @@
 diff --git a/internal/app/oauth.go b/internal/app/oauth.go
-index f0b9548..82f356f 100644
+index f0b9548..0c59a11 100644
 --- a/internal/app/oauth.go
 +++ b/internal/app/oauth.go
 @@ -162,12 +162,11 @@ func performChromedpOAuth(
@@ -7,7 +7,7 @@ index f0b9548..82f356f 100644
  			// Capture OAuth redirect
  			if strings.HasPrefix(reqURL, redirectPrefix) {
 -				log.Printf("[%s] Redirect URL: %s", requestID, reqURL)
-+				log.Printf("[%s] Captured OAuth redirect", requestID)
++				logCapturedOAuthRedirect(requestID, reqURL)
  				parsed, err := url.Parse(reqURL)
  				if err == nil {
  					if code := parsed.Query().Get("code"); code != "" {
@@ -21,7 +21,7 @@ index f0b9548..82f356f 100644
  				if parsed, perr := url.Parse(reqURL); perr == nil {
  					flowError = friendlyOPError(parsed.Query().Get("code"), parsed.Query().Get("message"))
 -					log.Printf("[%s] Stellantis error page: %s", requestID, reqURL)
-+					log.Printf("[%s] Stellantis error page received", requestID)
++					logStellantisErrorPage(requestID, reqURL)
  				}
  			} else if debug != nil && isRelevantURL(reqURL) {
  				// Only show relevant OAuth flow URLs in debug output
@@ -30,12 +30,65 @@ index f0b9548..82f356f 100644
  	var currentURL string
  	_ = chromedp.Run(browserCtx, chromedp.Location(&currentURL))
 -	log.Printf("Current URL: %s", currentURL)
-+	log.Printf("Current browser location checked")
++	logCurrentBrowserLocation(currentURL)
  	if !strings.HasPrefix(currentURL, redirectPrefix) {
  		return ""
  	}
+@@ -462,3 +461,15 @@ func codeFromLocation(browserCtx context.Context, redirectPrefix string) string
+ 	}
+ 	return parsed.Query().Get("code")
+ }
++
++func logCapturedOAuthRedirect(requestID, _ string) {
++	log.Printf("[%s] Captured OAuth redirect", requestID)
++}
++
++func logStellantisErrorPage(requestID, _ string) {
++	log.Printf("[%s] Stellantis error page received", requestID)
++}
++
++func logCurrentBrowserLocation(_ string) {
++	log.Printf("Current browser location checked")
++}
+diff --git a/internal/app/oauth_test.go b/internal/app/oauth_test.go
+index 3589572..b9ad90d 100644
+--- a/internal/app/oauth_test.go
++++ b/internal/app/oauth_test.go
+@@ -6,6 +6,32 @@ import (
+ 	"testing"
+ )
+ 
++func TestOAuthEventLogsDoNotContainURLsCodesOrTokens(t *testing.T) {
++	cookie := "event-cookie"
++	accessToken := "event-access-token"
++	refreshToken := "event-refresh-token"
++	redirectURL := "mypeugeot://oauth2redirect/dk?code=event-oauth-code&access_token=" + accessToken
++	errorURL := "https://idpcvs.peugeot.com/am/oauth2/OPErrorPage.php?cookie=" + cookie
++	currentURL := redirectURL + "&refresh_token=" + refreshToken
++	oauthCode := "event-oauth-code"
++
++	logs := captureLogs(t, func() {
++		logCapturedOAuthRedirect("redirect-request-id", redirectURL)
++		logStellantisErrorPage("error-request-id", errorURL)
++		logCurrentBrowserLocation(currentURL)
++	})
++	want := "[redirect-request-id] Captured OAuth redirect\n" +
++		"[error-request-id] Stellantis error page received\n" +
++		"Current browser location checked\n"
++	if logs != want {
++		t.Fatalf("logs = %q, want %q", logs, want)
++	}
++	assertLogRedacted(t, logs,
++		cookie, accessToken, refreshToken,
++		redirectURL, errorURL, currentURL, oauthCode,
++	)
++}
++
+ func TestFriendlyOPError(t *testing.T) {
+ 	cases := []struct {
+ 		name    string
 diff --git a/internal/app/server.go b/internal/app/server.go
-index 48a487e..98accf9 100644
+index 48a487e..946d8cf 100644
 --- a/internal/app/server.go
 +++ b/internal/app/server.go
 @@ -121,6 +121,13 @@ func getClientIP(r *http.Request) string {
@@ -52,7 +105,22 @@ index 48a487e..98accf9 100644
  // refundIfExpired gives back the rate-limit charge when the OAuth attempt
  // failed with a transient "session expired" error (bounded by the limiter).
  func refundIfExpired(clientIP, requestID string, err error) {
-@@ -136,7 +143,7 @@ func handleOAuth(w http.ResponseWriter, r *http.Request) {
+@@ -129,6 +136,14 @@ func refundIfExpired(clientIP, requestID string, err error) {
+ 	}
+ }
+ 
++func logOAuthRequest(requestID, clientIP string, req OAuthRequest) {
++	log.Printf("[%s] OAuth request from %s (%s/%s)", requestID, clientIP, req.Brand, req.Country)
++}
++
++func logOAuthFailure(requestID string, _ error) {
++	log.Printf("[%s] OAuth failed", requestID)
++}
++
+ func handleOAuth(w http.ResponseWriter, r *http.Request) {
+ 	if r.Method != http.MethodPost {
+ 		sendError(w, "Method not allowed", http.StatusMethodNotAllowed)
+@@ -136,7 +151,7 @@ func handleOAuth(w http.ResponseWriter, r *http.Request) {
  	}
  
  	// Get client IP early for rate limiting
@@ -61,48 +129,49 @@ index 48a487e..98accf9 100644
  
  	// Check rate limit
  	if !rateLimiter.isAllowed(clientIP) {
-@@ -160,7 +167,7 @@ func handleOAuth(w http.ResponseWriter, r *http.Request) {
+@@ -160,7 +175,7 @@ func handleOAuth(w http.ResponseWriter, r *http.Request) {
  	// Generate request ID
  	requestID := uuid.New().String()
  
 -	log.Printf("[%s] OAuth request from %s for user %s (%s/%s)", requestID, clientIP, req.Email, req.Brand, req.Country)
-+	log.Printf("[%s] OAuth request from %s (%s/%s)", requestID, clientIP, req.Brand, req.Country)
++	logOAuthRequest(requestID, clientIP, req)
  
  	// Check if client accepts SSE
  	if r.Header.Get("Accept") == "text/event-stream" {
-@@ -171,7 +178,7 @@ func handleOAuth(w http.ResponseWriter, r *http.Request) {
+@@ -171,7 +186,7 @@ func handleOAuth(w http.ResponseWriter, r *http.Request) {
  	code, err := performOAuth(req, requestID, nil, nil)
  	if err != nil {
  		refundIfExpired(clientIP, requestID, err)
 -		log.Printf("[%s] OAuth failed: %s", requestID, err.Error())
-+		log.Printf("[%s] OAuth failed", requestID)
++		logOAuthFailure(requestID, err)
  		sendError(w, err.Error(), http.StatusBadRequest)
  		return
  	}
-@@ -206,7 +213,7 @@ func handleOAuthSSE(w http.ResponseWriter, req OAuthRequest, requestID, clientIP
+@@ -206,7 +221,7 @@ func handleOAuthSSE(w http.ResponseWriter, req OAuthRequest, requestID, clientIP
  	code, err := performOAuth(req, requestID, progress, debug)
  	if err != nil {
  		refundIfExpired(clientIP, requestID, err)
 -		log.Printf("[%s] OAuth failed: %s", requestID, err.Error())
-+		log.Printf("[%s] OAuth failed", requestID)
++		logOAuthFailure(requestID, err)
  		_, _ = fmt.Fprintf(w, "data: {\"type\":\"error\",\"message\":\"%s\"}\n\n", err.Error())
  		flusher.Flush()
  		return
 diff --git a/internal/app/server_test.go b/internal/app/server_test.go
-index b56bb27..df05e37 100644
+index b56bb27..c9ab68e 100644
 --- a/internal/app/server_test.go
 +++ b/internal/app/server_test.go
-@@ -1,7 +1,9 @@
+@@ -1,7 +1,10 @@
  package app
  
  import (
 +	"bytes"
  	"encoding/json"
++	"errors"
 +	"log"
  	"net/http"
  	"net/http/httptest"
  	"os"
-@@ -188,3 +190,56 @@ func TestParseClientIP(t *testing.T) {
+@@ -188,3 +191,74 @@ func TestParseClientIP(t *testing.T) {
  		}
  	}
  }
@@ -119,48 +188,66 @@ index b56bb27..df05e37 100644
 +	}
 +}
 +
-+func TestRequestLogsDoNotContainCredentialsOrURLs(t *testing.T) {
++func captureLogs(t *testing.T, fn func()) string {
-+	sentinels := []string{
++	t.Helper()
-+		"sentinel-email@example.invalid",
-+		"sentinel-password",
-+		"https://idpcvs.peugeot.com/am/oauth2/authorize?sentinel=authorize-url",
-+		"mypeugeot://oauth2redirect/dk?sentinel=redirect-url",
-+		"sentinel-oauth-code",
-+	}
-+
 +	var logs bytes.Buffer
 +	originalWriter := log.Writer()
 +	originalFlags := log.Flags()
 +	log.SetOutput(&logs)
 +	log.SetFlags(0)
-+	t.Cleanup(func() {
++	defer func() {
 +		log.SetOutput(originalWriter)
 +		log.SetFlags(originalFlags)
-+	})
++	}()
 +
-+	body, err := json.Marshal(OAuthRequest{
++	fn()
-+		Brand:    "MyPeugeot",
++	return logs.String()
-+		Country:  "NOT-A-COUNTRY",
-+		Email:    strings.Join(sentinels, "|"),
-+		Password: "sentinel-password",
-+	})
-+	if err != nil {
-+		t.Fatalf("marshal request: %v", err)
 +}
-+	req := httptest.NewRequest(http.MethodPost, "/oauth", bytes.NewReader(body))
-+	req.RemoteAddr = "192.0.2.10:1234"
-+	w := httptest.NewRecorder()
-+
-+	handleOAuth(w, req)
 +
++func assertLogRedacted(t *testing.T, logs string, sentinels ...string) {
++	t.Helper()
 +	for _, sentinel := range sentinels {
-+		if strings.Contains(logs.String(), sentinel) {
++		if strings.Contains(logs, sentinel) {
-+			t.Errorf("logs contain sensitive value %q: %s", sentinel, logs.String())
++			t.Errorf("logs contain sensitive value %q: %s", sentinel, logs)
 +		}
 +	}
 +}
++
++func TestOAuthRequestAndFailureLogsDoNotContainSensitiveValues(t *testing.T) {
++	email := "sentinel-email@example.invalid"
++	password := "sentinel-password"
++	cookie := "sentinel-cookie"
++	accessToken := "sentinel-access-token"
++	refreshToken := "sentinel-refresh-token"
++	authorizeURL := "https://idpcvs.peugeot.com/am/oauth2/authorize?sentinel=authorize-url"
++	redirectURL := "mypeugeot://oauth2redirect/dk?sentinel=redirect-url"
++	oauthCode := "sentinel-oauth-code"
++	req := OAuthRequest{
++		Brand:    "MyPeugeot",
++		Country:  "DK",
++		Email:    email,
++		Password: password,
++	}
++	failure := errors.New(strings.Join([]string{
++		cookie, accessToken, refreshToken, authorizeURL, redirectURL, oauthCode,
++	}, "|"))
++
++	logs := captureLogs(t, func() {
++		logOAuthRequest("request-id", "192.0.2.10", req)
++		logOAuthFailure("request-id", failure)
++	})
++	want := "[request-id] OAuth request from 192.0.2.10 (MyPeugeot/DK)\n" +
++		"[request-id] OAuth failed\n"
++	if logs != want {
++		t.Fatalf("logs = %q, want %q", logs, want)
++	}
++	assertLogRedacted(t, logs,
++		email, password, cookie, accessToken, refreshToken,
++		authorizeURL, redirectURL, oauthCode,
++	)
++}
 diff --git a/internal/app/worker.go b/internal/app/worker.go
-index 00b9cf8..0b6e6fd 100644
+index 00b9cf8..3dbe54a 100644
 --- a/internal/app/worker.go
 +++ b/internal/app/worker.go
 @@ -7,9 +7,20 @@ import (
@@ -224,17 +311,17 @@ index 00b9cf8..0b6e6fd 100644
  
  	requestID := uuid.New().String()
 -	log.Printf("[%s] worker OAuth request from %s for user %s", requestID, clientIP, req.Email)
-+	log.Printf("[%s] worker OAuth request from %s", requestID, clientIP)
++	logWorkerOAuthRequest(requestID, clientIP, req)
  
  	code, err := performChromedpOAuth(req.URL, req.Email, req.Password, scheme, requestID, nil, nil)
  	if err != nil {
  		refundIfExpired(clientIP, requestID, err)
 -		log.Printf("[%s] worker OAuth failed: %s", requestID, err.Error())
-+		log.Printf("[%s] worker OAuth failed", requestID)
++		logWorkerOAuthFailure(requestID, err)
  		sendWorkerError(w, err.Error(), http.StatusBadRequest)
  		return
  	}
-@@ -80,6 +102,23 @@ func handleWorker(w http.ResponseWriter, r *http.Request) {
+@@ -80,6 +102,31 @@ func handleWorker(w http.ResponseWriter, r *http.Request) {
  	_ = json.MarshalWrite(w, workerCode{Code: code})
  }
  
@@ -254,18 +341,66 @@ index 00b9cf8..0b6e6fd 100644
 +	}
 +	return nil
 +}
++
++func logWorkerOAuthRequest(requestID, clientIP string, _ workerRequest) {
++	log.Printf("[%s] worker OAuth request from %s", requestID, clientIP)
++}
++
++func logWorkerOAuthFailure(requestID string, _ error) {
++	log.Printf("[%s] worker OAuth failed", requestID)
++}
 +
  // redirectScheme extracts the custom redirect scheme (e.g. "mymap") from the
  // authorize URL's redirect_uri query parameter; the code-capture listener keys on
  // "<scheme>://".
 diff --git a/internal/app/worker_test.go b/internal/app/worker_test.go
-index e64964d..7870e47 100644
+index e64964d..2a05a3b 100644
 --- a/internal/app/worker_test.go
 +++ b/internal/app/worker_test.go
-@@ -8,6 +8,41 @@ import (
+@@ -2,12 +2,81 @@ package app
+ 
+ import (
+ 	"encoding/json"
++	"errors"
+ 	"net/http"
+ 	"net/http/httptest"
+ 	"strings"
  	"testing"
  )
  
++func TestWorkerRequestAndFailureLogsDoNotContainSensitiveValues(t *testing.T) {
++	email := "worker-email@example.invalid"
++	password := "worker-password"
++	cookie := "worker-cookie"
++	accessToken := "worker-access-token"
++	refreshToken := "worker-refresh-token"
++	authorizeURL := "https://idpcvs.peugeot.com/am/oauth2/authorize?sentinel=worker-authorize-url"
++	redirectURL := "mypeugeot://oauth2redirect/dk?sentinel=worker-redirect-url"
++	oauthCode := "worker-oauth-code"
++	req := workerRequest{
++		URL:      authorizeURL,
++		Email:    email,
++		Password: password,
++	}
++	failure := errors.New(strings.Join([]string{
++		cookie, accessToken, refreshToken, redirectURL, oauthCode,
++	}, "|"))
++
++	logs := captureLogs(t, func() {
++		logWorkerOAuthRequest("worker-request-id", "192.0.2.20", req)
++		logWorkerOAuthFailure("worker-request-id", failure)
++	})
++	want := "[worker-request-id] worker OAuth request from 192.0.2.20\n" +
++		"[worker-request-id] worker OAuth failed\n"
++	if logs != want {
++		t.Fatalf("logs = %q, want %q", logs, want)
++	}
++	assertLogRedacted(t, logs,
++		email, password, cookie, accessToken, refreshToken,
++		authorizeURL, redirectURL, oauthCode,
++	)
++}
++
 +func TestValidateWorkerURL(t *testing.T) {
 +	allowed := []string{
 +		"https://idpcvs.citroen.com/am/oauth2/authorize?redirect_uri=mycitroen%3A%2F%2Foauth2redirect%2Fdk",
@@ -304,7 +439,7 @@ index e64964d..7870e47 100644
  func TestHandleWorker_MethodNotAllowed(t *testing.T) {
  	req := httptest.NewRequest(http.MethodGet, "/worker", nil)
  	w := httptest.NewRecorder()
-@@ -30,6 +65,25 @@ func TestHandleWorker_InvalidBody(t *testing.T) {
+@@ -30,6 +99,25 @@ func TestHandleWorker_InvalidBody(t *testing.T) {
  	}
  }
  
@@ -330,7 +465,7 @@ index e64964d..7870e47 100644
  func TestHandleWorker_MissingParams(t *testing.T) {
  	body := `{"url":"","email":"","password":""}`
  	req := httptest.NewRequest(http.MethodPost, "/worker", strings.NewReader(body))
-@@ -83,7 +137,7 @@ func TestRedirectScheme(t *testing.T) {
+@@ -83,7 +171,7 @@ func TestRedirectScheme(t *testing.T) {
  		},
  		{
  			name:    "opel custom scheme",