fix(api): drop dead auth knobs, require https sign-out URLs, generic decode errors (FIX-AUTH-P3)

docs/runbooks/install.md

@@ -390,7 +390,8 @@ back in. So, by default, sign-out continues at the provider
 - The target comes only from the discovery document and the chart values.
   Nothing in the sign-out request (query, body, headers, `Host`) can change
   it, so it is not an open redirect. A discovered endpoint that is not an
-  absolute `http(s)` URL is ignored and sign-out stays local.
+  absolute `https` URL — `http` is accepted only for loopback dev hosts —
+  is ignored and sign-out stays local.
 
 ```yaml
 oidc:

docs/security/threat-model.md

@@ -146,8 +146,9 @@ Controls:
   verifier are bound to the initiating browser (SEC-03).
 - **Portal session** — server-side, opaque cookie value; rows keyed by
   SHA-256 digest of the session ID so a DB/backup read never yields a usable
-  cookie (`internal/store/sessions.go:23,63,102`). Sliding 30 min idle,
-  12 h absolute cap (`auth.go:133-137` config defaults); the backend extends
+  cookie (`internal/store/sessions.go:23,63,102`). Sliding 30 min idle
+  (`-session-idle`/`TCDI_SESSION_IDLE` → `store.NewSessionStore`), 12 h
+  absolute cap (`AuthConfig.AbsoluteTimeout`); the backend extends
   idle on server-measured input activity. The retained `id_token` is sealed
   at rest with the same keys under a purpose-bound AAD
   (`internal/api/idtoken.go`); on read failure logout degrades to a
@@ -233,8 +234,9 @@ PKCE means a secretless public client also works), RP-initiated logout.
 Controls: exact issuer/audience match via go-oidc; `endSessionURL` is built
 only from discovery + static configuration so request input cannot produce an
 open redirect (`internal/api/auth.go:770-787`);
-`PostLogoutRedirect` is validated as an absolute http(s) URL at startup
-(auth.go:359). Client secret comes from `oidc.existingSecret` (chart).
+`PostLogoutRedirect` is validated as an absolute https URL at startup
+(auth.go) — http is accepted only for loopback dev hosts. Client secret
+comes from `oidc.existingSecret` (chart).
 Tests: `auth_test.go`, `logout_test.go`, `oidctest/` in-memory IdP.
 
 Open question: A6-S2 — replay of the sealed login cookie within its TTL
@@ -523,6 +525,9 @@ items that landed since.
 | S22 | Deploy-time guards for hazardous value combinations | Implemented (#119): `tinycdi.validate` now fails the render on `operator.leaderElect=false` with `operator.replicas>1` — unelected replicas would double-drive reconciliation and the binary cannot observe the Deployment's replica count (`templates/_helpers.tpl`, `cmd/operator` note; `TestOperatorLeaderElectionGuard`). Metrics-port isolation is pinned for every `edgeIngress` mode: only `allow-metrics-scrape` opens :9090, to exactly `networkPolicy.prometheusPeers` (`TestMetricsListenerIsolation`, `TestEdgePolicyNeverOpensInternalPort`). Residual: single-replica installs (`replicas: 1`) may still run unelected, and guards only constrain chart-rendered manifests — hand-rolled manifests bypass them |
 | S23 | Postgres-only restore onto a live cluster (DR) | Implemented (#122 runbook + #124 tool): `docs/runbooks/disaster-recovery.md` §"Postgres-only restore onto a live cluster" enumerates what an older dump resurrects — `sessions` rows including post-dump sign-outs (killed by `platform_meta.session_epoch` rotation — `SessionStore.Get` compares epoch in the same read), `'active'` `connection_lease` rows including post-dump revokes (a non-NULL `portal_session_digest` dies at next renew/attach via the S17 check, but the tool revokes every restored lease unconditionally anyway — NULL-digest rows have no barrier), unconsumed `launch_ticket` rows re-arming (denied; redeem also re-checks the session), and `workspaces` rows trailing live CRs (aligned to `spec.desiredState`/`runtimeGeneration`/`intentRevision` so the applied-intent fence is not bypassed). `backend post-restore` is the one-shot implementation: dry-run by default; `-apply` requires `-i-have-scaled-down`, takes the leader advisory lock on a dedicated connection and holds it for the whole run (a serving replica or concurrent run means refusal with zero writes), refuses while backend connections remain in `pg_stat_activity` unless `-i-know-backends-are-running`, and exits non-zero printing per-workspace SQL when Kubernetes read access for CR alignment is absent (`internal/backend/postrestore.go`; tests `postrestore_test.go`, `TestSessionEpochRotation`). The `hack/quickstart/restore-drill.sh` kind drill is manual, not a per-PR gate |
 | S24 | Intent-stream drift vs the applied-intent fence | Implemented (#125): after a DB restore the platform's `workspaces.intent_revision` can trail the fence the CR already recorded, and every new intent would be dropped as stale — silently. The applier now detects the two unreachable-by-replay shapes (strictly-behind revision; equal revision with diverged `desiredState`/`runtimeGeneration`) and stamps `workspaces.cdi.tinyorbit.vn/intent-behind` `{rowRevision, crRevision}` (`intentFenceDrifted`, `internal/provisioning/k8sapplier.go`); the operator raises `ConditionIntentBehind` with the revision params plus one edge-triggered Warning event, and clears it once the stream applies forward again (`internal/operator/workspace_controller.go`, `api/v1alpha1/validation.go`; events `create`/`patch` RBAC added). The fence itself is unchanged — a stale intent is still never applied; drift is only surfaced. Surfaced to users via the conditions/events projection (`statusview.go`, `events.go`; `TestIntentBehindDrift`, `TestK8sApplierIntentDrift`, `TestProjectConditions_IntentBehind`) |
+| S25 | Dead/misleading auth config knobs | Fixed: `AuthConfig` carried `IdleTimeout` — defaulted at startup but never consumed (the session idle window is owned by the session store: `-session-idle`/`TCDI_SESSION_IDLE` → `store.NewSessionStore`) — and `AllowedTenants` — enforced in `tenantAllowed` but with no flag/env/chart path able to populate it, so the gate could never engage; `-required-groups` (`oidc.requiredGroups`) remains the supported login gate. Both knobs were deleted rather than wired: wiring `IdleTimeout` would have created a second source of truth for the store's window, and `AllowedTenants` had no reachable configuration. Guard: `TestAuthConfig_NoDeadSessionKnobs` fails if either field returns |
+| S26 | Post-logout redirect accepted plaintext `http` | Fixed: `AuthConfig.PostLogoutRedirect` and the discovered `end_session_endpoint` now require an absolute https URL — `http` is accepted only for loopback hosts (`localhost`, 127.0.0.0/8, `::1`; strict `netip` parse — mapped/octal spellings do not count), matching the dev plain-http-on-loopback IdP convention (`oidctest`); the chart schema already required `^https://` (`values.schema.json`). Tests: `TestNewAuthenticator_RejectsBadPostLogoutRedirect`, `TestNewAuthenticator_AllowsLoopbackPostLogoutRedirect`, `TestLogout_UntrustedDiscoveredEndpointIs204` |
+| S27 | create-connection echoed JSON decoder detail | Fixed: `ConnectionHandler.Create` decodes via the shared `decodeJSON` helper — which also rejects trailing data after the first document, a check the previous inline decode lacked — answers 400 `INVALID_REQUEST` with the generic "invalid request body", and logs the decode detail server-side with the request id (SEC-I7). `decodeJSON` now returns the error so callers can log it without echoing it. Tests: `TestCreateConnection_InvalidBodyGenericMessage`, `TestCreateConnection_TrailingJSONRejected` |
 
 *Review note — S17:* the ticket-lock serialization claim (a redeem's
 ticket-row `FOR UPDATE` vs the revoke's `UPDATE` under READ COMMITTED) is

internal/api/adminquota.go

@@ -345,7 +345,7 @@ func (h *AdminQuotaHandler) Put(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	var req adminQuotaLimits
-	if !decodeJSON(body, &req) {
+	if decodeJSON(body, &req) != nil {
 		writeError(w, r, CodeInvalidRequest, "invalid request body")
 		return
 	}

internal/api/adminuserlimit.go

@@ -227,7 +227,7 @@ func (h *AdminUserLimitsHandler) decodeBody(w http.ResponseWriter, r *http.Reque
 		writeError(w, r, CodeInvalidRequest, "unreadable or oversized body")
 		return false
 	}
-	if !decodeJSON(body, v) {
+	if decodeJSON(body, v) != nil {
 		writeError(w, r, CodeInvalidRequest, "invalid request body")
 		return false
 	}

internal/api/auth.go

@@ -14,6 +14,7 @@ import (
 	"fmt"
 	"log/slog"
 	"net/http"
+	"net/netip"
 	"net/url"
 	"strconv"
 	"strings"
@@ -67,10 +68,11 @@ type AuthConfig struct {
 	// set it false for httptest servers, release builds must not.
 	SecureCookies *bool
 
-	// IdleTimeout is the sliding inactivity lifetime of a session
-	// (default 30m). AbsoluteTimeout is the hard cap from creation
-	// (default 12h).
-	IdleTimeout     time.Duration
+	// AbsoluteTimeout is the hard cap on a session's lifetime from
+	// creation (default 12h). The sliding idle timeout is not configured
+	// here: it is owned by the session store (-session-idle /
+	// TCDI_SESSION_IDLE -> store.NewSessionStore), which enforces it on
+	// every read.
 	AbsoluteTimeout time.Duration
 	// PendingTTL bounds how long a login attempt (state/nonce/PKCE
 	// verifier) stays valid (default 10m). It is sealed into the login
@@ -86,9 +88,6 @@ type AuthConfig struct {
 	// membership and group membership. Defaults: "tenant_id", "groups".
 	TenantClaim string
 	GroupsClaim string
-	// AllowedTenants, when non-empty, restricts login to principals whose
-	// TenantID is listed. TenantID empty is always rejected.
-	AllowedTenants []string
 	// RequiredGroups, when non-empty, additionally restricts login to
 	// principals whose Groups claim contains at least one listed group.
 	// The match is exact — the Keycloak full-path form "/platform-admins"
@@ -108,7 +107,7 @@ type AuthConfig struct {
 	// end-session URL (chart oidc.postLogoutRedirect). Empty omits it: the
 	// URI must be registered at the provider, so it is opt-in and the
 	// provider then shows its own logged-out page. Must be an absolute
-	// http(s) URL.
+	// https URL (http only for loopback dev hosts).
 	PostLogoutRedirect string
 }
 
@@ -129,9 +128,6 @@ func (c *AuthConfig) withDefaults() {
 		t := true
 		c.SecureCookies = &t
 	}
-	if c.IdleTimeout == 0 {
-		c.IdleTimeout = 30 * time.Minute
-	}
 	if c.AbsoluteTimeout == 0 {
 		c.AbsoluteTimeout = 12 * time.Hour
 	}
@@ -359,8 +355,8 @@ func NewAuthenticator(ctx context.Context, cfg AuthConfig, sessions SessionStore
 	if log == nil {
 		log = slog.Default()
 	}
-	if cfg.PostLogoutRedirect != "" && !isAbsoluteHTTPURL(cfg.PostLogoutRedirect) {
-		return nil, errors.New("api: AuthConfig PostLogoutRedirect must be an absolute http(s) URL")
+	if cfg.PostLogoutRedirect != "" && !isHTTPSOrLoopbackURL(cfg.PostLogoutRedirect) {
+		return nil, errors.New("api: AuthConfig PostLogoutRedirect must be an absolute https URL (http only for loopback hosts)")
 	}
 	provider, err := oidc.NewProvider(ctx, cfg.Issuer)
 	if err != nil {
@@ -374,10 +370,10 @@ func NewAuthenticator(ctx context.Context, cfg AuthConfig, sessions SessionStore
 		if err := provider.Claims(&disc); err != nil {
 			return nil, fmt.Errorf("api: OIDC discovery claims: %w", err)
 		}
-		if isAbsoluteHTTPURL(disc.EndSessionEndpoint) {
+		if isHTTPSOrLoopbackURL(disc.EndSessionEndpoint) {
 			endSession = disc.EndSessionEndpoint
 		} else if disc.EndSessionEndpoint != "" {
-			log.Warn("oidc end_session_endpoint ignored: not an absolute http(s) URL without credentials")
+			log.Warn("oidc end_session_endpoint ignored: not an absolute https URL (http only for loopback hosts) without credentials")
 		}
 	}
 	a := &Authenticator{
@@ -576,7 +572,7 @@ func (a *Authenticator) CallbackHandler(w http.ResponseWriter, r *http.Request)
 		DisplayName: displayNameClaim(claims),
 		Email:       displayClaim(stringClaim(claims, "email"), maxEmailLen),
 	}
-	if principal.TenantID == "" || !a.tenantAllowed(principal.TenantID) {
+	if principal.TenantID == "" {
 		writeError(w, r, CodeForbidden, "no tenant membership for this account")
 		return
 	}
@@ -812,14 +808,32 @@ func (a *Authenticator) endSessionURL(idToken string) string {
 	return u.String()
 }
 
-// isAbsoluteHTTPURL reports whether s is an absolute http(s) URL with a host
-// and no embedded credentials.
-func isAbsoluteHTTPURL(s string) bool {
+// isHTTPSOrLoopbackURL reports whether s is an absolute URL the sign-out
+// flow may hand to the browser: https always; http only when the host is
+// loopback (localhost / 127.0.0.0/8 / ::1) — the codebase's dev convention
+// is a plain-http IdP on loopback (oidctest, a dev Keycloak), and plaintext
+// to anywhere else is never a legitimate navigation target. Embedded
+// credentials are rejected.
+func isHTTPSOrLoopbackURL(s string) bool {
 	u, err := url.Parse(s)
 	if err != nil || u.Host == "" || u.User != nil {
 		return false
 	}
-	return u.Scheme == "https" || u.Scheme == "http"
+	if u.Scheme == "https" {
+		return true
+	}
+	return u.Scheme == "http" && isLoopbackHost(u.Hostname())
+}
+
+// isLoopbackHost reports whether host names a loopback address: the literal
+// "localhost" or an IP in the loopback range. Strict parsing — non-canonical
+// IP spellings (octal, hex, v4-in-v6 mapped) never count as loopback.
+func isLoopbackHost(host string) bool {
+	if strings.EqualFold(host, "localhost") {
+		return true
+	}
+	ip, err := netip.ParseAddr(host)
+	return err == nil && ip.Unmap().IsLoopback()
 }
 
 // sessionCookie builds the host-only session cookie: no Domain attribute
@@ -921,18 +935,6 @@ type throttleEntry struct {
 	at        time.Time
 }
 
-func (a *Authenticator) tenantAllowed(tenant string) bool {
-	if len(a.cfg.AllowedTenants) == 0 {
-		return true
-	}
-	for _, t := range a.cfg.AllowedTenants {
-		if t == tenant {
-			return true
-		}
-	}
-	return false
-}
-
 // groupsAllowed enforces RequiredGroups: with none configured the gate is
 // off; otherwise at least one required group must appear verbatim in the
 // verified claim. A missing or malformed claim parses to no groups and is

internal/api/auth_test.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"reflect"
 	"strings"
 	"sync"
 	"testing"
@@ -426,7 +427,6 @@ func TestSessionFixationPrevented(t *testing.T) {
 
 func TestSessionIdleAndAbsoluteExpiry(t *testing.T) {
 	env := newTestEnv(t, func(c *AuthConfig) {
-		c.IdleTimeout = time.Minute
 		c.AbsoluteTimeout = 10 * time.Minute
 	})
 	fc := &fakeClock{now: time.Now()}
@@ -477,10 +477,17 @@ func TestTenantMembershipRequired(t *testing.T) {
 	}
 }
 
-func TestTenantNotInAllowlistRejected(t *testing.T) {
-	env := newTestEnv(t, func(c *AuthConfig) { c.AllowedTenants = []string{"tenant-b"} })
-	if status, code := callbackStatus(t, env); status != http.StatusForbidden || code != string(CodeForbidden) {
-		t.Fatalf("status=%d code=%q", status, code)
+// AuthConfig must not grow knobs that cannot be wired to a flag: an
+// operator-facing field that is parsed but never populated or consumed is a
+// misleading security surface. The session idle window belongs to the
+// session store (-session-idle / TCDI_SESSION_IDLE); the login gate is
+// -required-groups. Keep the struct free of both dead fields.
+func TestAuthConfig_NoDeadSessionKnobs(t *testing.T) {
+	typ := reflect.TypeOf(AuthConfig{})
+	for _, field := range []string{"IdleTimeout", "AllowedTenants"} {
+		if _, ok := typ.FieldByName(field); ok {
+			t.Fatalf("AuthConfig.%s is a dead knob — wire it to a flag or remove it", field)
+		}
 	}
 }
 

internal/api/connections.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"io"
+	"log/slog"
 	"net/http"
 	"time"
 
@@ -55,6 +56,7 @@ type ConnectionHandler struct {
 	// audit is the dedicated audit-event sink the route emits through
 	// (nil = no domain audit events).
 	audit observability.AuditSink
+	log   *slog.Logger
 }
 
 // NewConnectionHandler wires the handler. domain is the session domain the
@@ -66,9 +68,19 @@ func NewConnectionHandler(issuer ConnectionIssuer, tenants TenantResolver, domai
 		tenants: tenants,
 		domain:  domain,
 		maxBody: 16 << 10,
+		log:     slog.Default(),
 	}
 }
 
+// WithLogger attaches the logger the handler writes server-side detail to
+// (e.g. the request-body decode detail that never reaches the client).
+func (h *ConnectionHandler) WithLogger(l *slog.Logger) *ConnectionHandler {
+	if l != nil {
+		h.log = l
+	}
+	return h
+}
+
 // WithClipboardSource wires the resolver that supplies the workspace's
 // template clipboard policy for ticket recording (V3.24: the gateway
 // redirect re-asserts the client flags from what the ticket recorded).
@@ -133,10 +145,12 @@ func (h *ConnectionHandler) Create(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if len(bytes.TrimSpace(body)) > 0 {
-		dec := json.NewDecoder(bytes.NewReader(body))
-		dec.DisallowUnknownFields()
-		if err := dec.Decode(&req); err != nil {
-			writeError(w, r, CodeInvalidRequest, "invalid request body: "+err.Error())
+		// SEC-I7: decode detail is logged server-side and never echoed —
+		// the client gets the generic message every other handler uses.
+		if err := decodeJSON(body, &req); err != nil {
+			h.log.Warn("connection create: invalid request body",
+				"request_id", RequestIDFromContext(r.Context()), "err", err)
+			writeError(w, r, CodeInvalidRequest, "invalid request body")
 			return
 		}
 	}

internal/api/connections_test.go

@@ -65,7 +65,7 @@ func newConnectionEnv(t *testing.T, issuer ConnectionIssuer, opts ...func(*Conne
 	if err != nil {
 		t.Fatalf("sessionhost.ParseDomain: %v", err)
 	}
-	h := NewConnectionHandler(issuer, defaultTenants(), d)
+	h := NewConnectionHandler(issuer, defaultTenants(), d).WithLogger(logger)
 	for _, o := range opts {
 		o(h)
 	}
@@ -164,6 +164,61 @@ func TestCreateConnection_RequiresAuthAndCSRF(t *testing.T) {
 	}
 }
 
+// TestCreateConnection_InvalidBodyGenericMessage: a malformed body answers
+// 400 INVALID_REQUEST with the generic message every other handler uses —
+// the decoder detail is logged server-side, never echoed (SEC-I7).
+func TestCreateConnection_InvalidBodyGenericMessage(t *testing.T) {
+	fi := &fakeIssuer{}
+	env := newConnectionEnv(t, fi)
+	sess, csrf := login(t, env, "alice")
+
+	r := doReq(t, env, sess, csrf, http.MethodPost,
+		"/v1/workspaces/ws_00000000000000000000000001/connections",
+		`{"takeover":true,"attacker_field":1}`, nil)
+	defer r.Body.Close()
+	if r.StatusCode != http.StatusBadRequest {
+		b, _ := io.ReadAll(r.Body)
+		t.Fatalf("status=%d body=%s, want 400", r.StatusCode, b)
+	}
+	b, _ := io.ReadAll(r.Body)
+	var e Error
+	if err := json.Unmarshal(b, &e); err != nil {
+		t.Fatalf("error body not JSON: %q", b)
+	}
+	if e.Code != CodeInvalidRequest || e.Message != "invalid request body" {
+		t.Fatalf("code=%q message=%q, want INVALID_REQUEST + generic message", e.Code, e.Message)
+	}
+	if bytes.Contains(b, []byte("attacker_field")) {
+		t.Fatalf("decoder detail echoed to client: %s", b)
+	}
+	if !strings.Contains(env.logs.String(), "attacker_field") {
+		t.Fatalf("decoder detail missing from server log: %s", env.logs.String())
+	}
+	if fi.calls != 0 {
+		t.Fatal("issuer called on an invalid body")
+	}
+}
+
+// TestCreateConnection_TrailingJSONRejected: the shared decoder accepts
+// exactly one document — a second JSON value after the request object is a
+// 400 like everywhere else.
+func TestCreateConnection_TrailingJSONRejected(t *testing.T) {
+	fi := &fakeIssuer{}
+	env := newConnectionEnv(t, fi)
+	sess, csrf := login(t, env, "alice")
+
+	r := doReq(t, env, sess, csrf, http.MethodPost,
+		"/v1/workspaces/ws_00000000000000000000000001/connections",
+		`{"takeover":true} {"extra":true}`, nil)
+	defer r.Body.Close()
+	if r.StatusCode != http.StatusBadRequest {
+		t.Fatalf("status=%d, want 400", r.StatusCode)
+	}
+	if fi.calls != 0 {
+		t.Fatal("issuer called on trailing data")
+	}
+}
+
 // TestCreateConnection_ConnectionInUse: the broker's ErrConnectionInUse maps
 // to 409 CONNECTION_IN_USE per openapi.yaml.
 func TestCreateConnection_ConnectionInUse(t *testing.T) {

internal/api/data.go

@@ -368,7 +368,7 @@ func (h *DataHandler) Attach(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	var req attachDataRequest
-	if !decodeJSON(body, &req) {
+	if decodeJSON(body, &req) != nil {
 		writeError(w, r, CodeInvalidRequest, "invalid request body")
 		return
 	}
@@ -444,7 +444,7 @@ func (h *DataHandler) Purge(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	var req purgeDataRequest
-	if !decodeJSON(body, &req) {
+	if decodeJSON(body, &req) != nil {
 		writeError(w, r, CodeInvalidRequest, "invalid request body")
 		return
 	}