From 6cc07a42e29dea3863364b18ad678d05838b4006 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 23 Sep 2026 23:12:02 +0200 Subject: [PATCH] fix(08-09): match wristband OAuth byte contract to live-recorded PHP Recording the full mcp-lifecycle fixture against real isolated PHP (08-09-PLAN.md Task 2) uncovered three byte-level gaps between wristband's assumed contract and actual production PHP behavior: - Every explicit "Cache-Control: no-store" PHP sets is actually delivered as "no-store, private" (Laravel's session-cookie default merges "private" onto any explicit value); wristband's own default for unheadered JSON error responses is "no-cache, private" (matching the house convention already used elsewhere), not empty. - PHP's redirect responses (authorize success and every error redirect) render Symfony's default HTML redirect body with Content-Type "text/html; charset=utf-8"; Go's bare 302 with no body never matched. wristband/redirect_html.go ports that exact byte template, including PHP's htmlspecialchars(ENT_QUOTES) escaping (Go's html.EscapeString uses different quote entities). tide/normalize.go: isIDKey now also masks "_ids" plural array fields (e.g. collection_ids), a latent parity-corpus gap no prior fixture had exercised with a literal, non-empty, non-placeholder array value. --- tide/normalize.go | 9 ++++- wristband/authorize.go | 16 +++++---- wristband/authorize_test.go | 16 ++++----- wristband/redirect_html.go | 60 ++++++++++++++++++++++++++++++++++ wristband/register.go | 4 +-- wristband/registration_test.go | 8 ++--- wristband/token.go | 8 +++-- wristband/token_test.go | 12 +++---- 8 files changed, 103 insertions(+), 30 deletions(-) create mode 100644 wristband/redirect_html.go diff --git a/tide/normalize.go b/tide/normalize.go index dbfee0b..c4e8edb 100644 --- a/tide/normalize.go +++ b/tide/normalize.go @@ -117,7 +117,14 @@ func isIDKey(key string) bool { if key == "id" { return true } - return strings.HasSuffix(key, "_id") && !strings.HasSuffix(key, "_at") + if strings.HasSuffix(key, "_at") { + return false + } + // "_ids" covers plural raw-integer-array fields such as collection_ids: + // each array element still reaches maskLeaf individually (maskValue + // recurses into []any before calling maskLeaf), so this masks every + // element the same way a singular "_id" scalar would be masked. + return strings.HasSuffix(key, "_id") || strings.HasSuffix(key, "_ids") } func lastPathKey(path string) string { diff --git a/wristband/authorize.go b/wristband/authorize.go index cab9092..ffbabbf 100644 --- a/wristband/authorize.go +++ b/wristband/authorize.go @@ -164,16 +164,19 @@ func (s *Server) Authorize(w http.ResponseWriter, r *http.Request) { } spa := s.opts.Issuer + "/connect?request=" + rfc3986Escape(requestID) - w.Header().Set("Cache-Control", "no-store") - w.Header().Set("Location", spa) - w.WriteHeader(http.StatusFound) + // Laravel appends ", private" to every explicit Cache-Control this + // endpoint sets (session-cookie default merge); recorded PHP traffic is + // "no-store, private", never a bare "no-store" (D-04, live-recorded byte + // contract, 08-09-PLAN.md Task 2). + w.Header().Set("Cache-Control", "no-store, private") + writeRedirectHTML(w, http.StatusFound, spa) } // writeAuthorizeLocalError writes the PHP localError() response: a bare // text/plain 400 with no Location and no house envelope // (T-08-OPEN-REDIRECT). func writeAuthorizeLocalError(w http.ResponseWriter, message string) { - w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Cache-Control", "no-store, private") w.Header().Set("Content-Type", "text/plain; charset=UTF-8") w.WriteHeader(http.StatusBadRequest) _, _ = w.Write([]byte(message)) @@ -191,9 +194,8 @@ func (s *Server) authorizeErrorRedirect(w http.ResponseWriter, redirectURI, errC if state != nil { pairs = append(pairs, [2]string{"state", *state}) } - w.Header().Set("Cache-Control", "no-store") - w.Header().Set("Location", appendOrderedQuery(redirectURI, pairs)) - w.WriteHeader(http.StatusFound) + w.Header().Set("Cache-Control", "no-store, private") + writeRedirectHTML(w, http.StatusFound, appendOrderedQuery(redirectURI, pairs)) } // parseAuthorizeScopes ports OAuthAuthorizeController::parseScopes. An diff --git a/wristband/authorize_test.go b/wristband/authorize_test.go index 638c26d..614c772 100644 --- a/wristband/authorize_test.go +++ b/wristband/authorize_test.go @@ -113,8 +113,8 @@ func TestPhase8RedAuthorize(t *testing.T) { if _, has := q["code"]; has { t.Fatalf("PHASE8_RED:authorize: Location %q leaks a code onto our own redirect", loc) } - if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { - t.Fatalf("PHASE8_RED:authorize: Cache-Control = %q, want \"no-store\"", cc) + if cc := rec.Header().Get("Cache-Control"); cc != "no-store, private" { + t.Fatalf("PHASE8_RED:authorize: Cache-Control = %q, want \"no-store, private\"", cc) } } @@ -152,8 +152,8 @@ func TestAuthorizeUnknownClientReturnsLocal400NoLocation(t *testing.T) { if body := rec.Body.String(); body != "Unknown client." { t.Fatalf("body = %q, want %q", body, "Unknown client.") } - if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { - t.Fatalf("Cache-Control = %q, want no-store", cc) + if cc := rec.Header().Get("Cache-Control"); cc != "no-store, private" { + t.Fatalf("Cache-Control = %q, want no-store, private", cc) } } @@ -278,8 +278,8 @@ func TestPKCEChallengeMethodMustBeS256(t *testing.T) { if q["iss"] != "https://plytarium.com" { t.Fatalf("iss = %q, want https://plytarium.com", q["iss"]) } - if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { - t.Fatalf("Cache-Control = %q, want no-store", cc) + if cc := rec.Header().Get("Cache-Control"); cc != "no-store, private" { + t.Fatalf("Cache-Control = %q, want no-store, private", cc) } } @@ -358,8 +358,8 @@ func TestAuthorizeValidRequestRedirectsToConnectWithOpaqueHandleOnly(t *testing. if _, has := q["client_secret"]; has { t.Fatal("Location leaks client_secret") } - if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { - t.Fatalf("Cache-Control = %q, want no-store", cc) + if cc := rec.Header().Get("Cache-Control"); cc != "no-store, private" { + t.Fatalf("Cache-Control = %q, want no-store, private", cc) } backend.mu.Lock() diff --git a/wristband/redirect_html.go b/wristband/redirect_html.go new file mode 100644 index 0000000..f634eee --- /dev/null +++ b/wristband/redirect_html.go @@ -0,0 +1,60 @@ +package wristband + +import ( + "net/http" + "strings" +) + +// htmlEscapePHP ports PHP's htmlspecialchars($s, ENT_QUOTES, 'UTF-8') byte +// for byte: Go's stdlib html.EscapeString differs on the quote entities +// ('/" vs PHP's '/"), which would diverge from the +// recorded redirect body whenever a redirect_uri or state value contains a +// quote character. +func htmlEscapePHP(s string) string { + var b strings.Builder + b.Grow(len(s)) + for _, r := range s { + switch r { + case '&': + b.WriteString("&") + case '"': + b.WriteString(""") + case '\'': + b.WriteString("'") + case '<': + b.WriteString("<") + case '>': + b.WriteString(">") + default: + b.WriteRune(r) + } + } + return b.String() +} + +// writeRedirectHTML ports Laravel/Symfony's RedirectResponse default HTML +// body byte-for-byte (T-08-OPEN-REDIRECT/D-04). Go's net/http never emits a +// body for a 3xx Location redirect; every wristband redirect needs this +// exact body plus Content-Type because real recorded PHP traffic includes +// it, and an unchanged browser-based client (the Nuxt /connect handoff) +// observes it. Cache-Control is the caller's responsibility -- callers set +// it before invoking this helper because its value differs between the +// authorize success path and error redirects versus other endpoints. +func writeRedirectHTML(w http.ResponseWriter, status int, target string) { + escaped := htmlEscapePHP(target) + var b strings.Builder + b.WriteString("\n\n \n \n \n\n Redirecting to ") + b.WriteString(escaped) + b.WriteString("\n \n \n Redirecting to ") + b.WriteString(escaped) + b.WriteString(".\n \n") + + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Location", target) + w.WriteHeader(status) + _, _ = w.Write([]byte(b.String())) +} diff --git a/wristband/register.go b/wristband/register.go index 0dda298..d5ff453 100644 --- a/wristband/register.go +++ b/wristband/register.go @@ -166,11 +166,11 @@ func (s *Server) Register(w http.ResponseWriter, r *http.Request) { zero := int64(0) resp.ClientSecretExpiresAt = &zero } - writeExactJSON(w, http.StatusCreated, resp, map[string]string{"Cache-Control": "no-store"}) + writeExactJSON(w, http.StatusCreated, resp, map[string]string{"Cache-Control": "no-store, private"}) } func writeRegisterError(w http.ResponseWriter, status int, code, description string) { - writeExactJSON(w, status, rfcErrorBody{Error: code, ErrorDescription: description}, map[string]string{"Cache-Control": "no-store"}) + writeExactJSON(w, status, rfcErrorBody{Error: code, ErrorDescription: description}, map[string]string{"Cache-Control": "no-store, private"}) } // isJSONContentType mirrors Laravel's Request::isJson(): the Content-Type diff --git a/wristband/registration_test.go b/wristband/registration_test.go index 2321518..2bb9c5e 100644 --- a/wristband/registration_test.go +++ b/wristband/registration_test.go @@ -314,8 +314,8 @@ func TestPhase8RedRegistration(t *testing.T) { if _, hasSecret := got["client_secret"]; hasSecret { t.Fatal("PHASE8_RED:registration: public client response has a client_secret, want none") } - if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { - t.Fatalf("PHASE8_RED:registration: Cache-Control = %q, want \"no-store\"", cc) + if cc := rec.Header().Get("Cache-Control"); cc != "no-store, private" { + t.Fatalf("PHASE8_RED:registration: Cache-Control = %q, want \"no-store, private\"", cc) } } @@ -614,7 +614,7 @@ func assertRegisterError(t *testing.T, rec *httptest.ResponseRecorder, status in if got["error_description"] != description { t.Fatalf("error_description = %v, want %q", got["error_description"], description) } - if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { - t.Fatalf("Cache-Control = %q, want \"no-store\"", cc) + if cc := rec.Header().Get("Cache-Control"); cc != "no-store, private" { + t.Fatalf("Cache-Control = %q, want \"no-store, private\"", cc) } } diff --git a/wristband/token.go b/wristband/token.go index bad6969..f2f7c08 100644 --- a/wristband/token.go +++ b/wristband/token.go @@ -142,7 +142,7 @@ func (s *Server) Token(w http.ResponseWriter, r *http.Request) { Scope: scope, } writeExactJSON(w, http.StatusOK, body, map[string]string{ - "Cache-Control": "no-store", + "Cache-Control": "no-store, private", "Pragma": "no-cache", }) } @@ -409,5 +409,9 @@ func (s *Server) Revoke(ctx context.Context, apiTokenID uint) error { } func writeTokenError(w http.ResponseWriter, status int, code string) { - writeExactJSON(w, status, tokenErrorBody{Error: code}, nil) + // PHP's rfcError() sets no explicit Cache-Control; Laravel's own + // session-cookie default for an otherwise-unheadered JSON response is + // "no-cache, private" (matches the live-recorded byte contract, same + // default the house wire.WriteJSON convention already uses elsewhere). + writeExactJSON(w, status, tokenErrorBody{Error: code}, map[string]string{"Cache-Control": "no-cache, private"}) } diff --git a/wristband/token_test.go b/wristband/token_test.go index 239f67f..824d4ca 100644 --- a/wristband/token_test.go +++ b/wristband/token_test.go @@ -121,8 +121,8 @@ func TestPhase8RedCodeExchange(t *testing.T) { if got["scope"] != "read write" { t.Fatalf("PHASE8_RED:code-exchange: scope = %v, want %q", got["scope"], "read write") } - if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { - t.Fatalf("PHASE8_RED:code-exchange: Cache-Control = %q, want \"no-store\"", cc) + if cc := rec.Header().Get("Cache-Control"); cc != "no-store, private" { + t.Fatalf("PHASE8_RED:code-exchange: Cache-Control = %q, want \"no-store, private\"", cc) } } @@ -325,8 +325,8 @@ func TestTokenConfidentialClientWrongSecretIsInvalidClient(t *testing.T) { if wa := rec.Header().Get("WWW-Authenticate"); wa != `Basic realm="OAuth"` { t.Fatalf("WWW-Authenticate = %q, want %q", wa, `Basic realm="OAuth"`) } - if cc := rec.Header().Get("Cache-Control"); cc != "" { - t.Fatalf("Cache-Control = %q, want none on an error response", cc) + if cc := rec.Header().Get("Cache-Control"); cc != "no-cache, private" { + t.Fatalf("Cache-Control = %q, want %q", cc, "no-cache, private") } } @@ -558,8 +558,8 @@ func TestTokenSuccessResponseHasNoEnvelopeAndNoTrailingNewline(t *testing.T) { if got["expires_in"] != float64(3600) { t.Fatalf("expires_in = %v, want 3600", got["expires_in"]) } - if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { - t.Fatalf("Cache-Control = %q, want \"no-store\"", cc) + if cc := rec.Header().Get("Cache-Control"); cc != "no-store, private" { + t.Fatalf("Cache-Control = %q, want \"no-store, private\"", cc) } if p := rec.Header().Get("Pragma"); p != "no-cache" { t.Fatalf("Pragma = %q, want \"no-cache\"", p)