Merge pull request #298 from ipnet-mesh/fix/illegal-user-name-header

fix(web): sanitize X-User-Name header to prevent 502 on registration
2026-08-09 10:23:03 +02:00 · 2026-07-04 20:10:54 +01:00
parent dc72657b0a cbd12b6c25
commit 62d5b637f0
9 changed files with 647 additions and 4 deletions
@@ -0,0 +1,294 @@
+# Fix Illegal `X-User-Name` Header Value on New-User Registration
+
+**Date:** 2026-07-04
+**Status:** Proposed
+**Slug:** fix-illegal-user-name-header
+
+## Summary
+
+When an OIDC identity provider returns a `name` claim containing leading or
+trailing whitespace (e.g. `"Matt "`) at registration time, every authenticated
+proxied API request begins to fail with `502 API proxy error` and the log line
+`API proxy error: Illegal header value b'Matt '`. The dirty value flows
+unmodified from the IdP token, into the Starlette session, and is injected
+verbatim as the `X-User-Name` request header by the web proxy layer; `httpx`
+then rejects it under RFC 7230 (which forbids leading/trailing OWS and embedded
+control characters in header field values).
+
+This plan normalizes the display name at ingress and adds a defensive guard at
+the header-construction boundary so the proxy can never emit an illegal header
+value regardless of where the data originated. The fix is safe because
+`X-User-Name` is purely informational: its sole consumer seeds the non-unique
+`UserProfile.name` display column on first profile creation, and identity /
+authorization are keyed on `X-User-Id` (the OIDC `sub`) and `X-User-Roles`
+respectively.
+
+## Background & Motivation
+
+### Reported incident
+
+A new user signed up via the IdP, then attempted to update their profile name.
+The profile update (and in fact **all** subsequent authenticated API calls)
+returned `502`, and the collector log recorded:
+
+```
+meshcore_hub.web.app - ERROR - API proxy error: Illegal header value b'Matt '
+```
+
+### Root cause (verified by code trace)
+
+1. The IdP returned a `name` claim with a trailing space at registration.
+2. `strip_userinfo()` in `src/meshcore_hub/web/oidc.py:58` copies the claim
+   verbatim into the session dict — no sanitization.
+3. On every authenticated API call the web proxy injects the session name as a
+   request header at `src/meshcore_hub/web/app.py:751`:
+   `headers["X-User-Name"] = user["name"]`.
+4. `httpx.AsyncClient.request(...)` enforces RFC 7230 and rejects the value,
+   raising an exception that is swallowed by the generic handler at
+   `src/meshcore_hub/web/app.py:793`, returning `502 {"detail": "API proxy error"}`.
+5. The same dirty value is injected at the auth-callback bootstrap
+   (`src/meshcore_hub/web/app.py:1128`), so the user's `UserProfile` is never
+   seeded on first login either.
+
+### Safety analysis — `X-User-Name` is informational only
+
+A full sweep of every `request.headers.get(...)` call in `src/` confirms the
+**only** reader of `X-User-Name` is
+`src/meshcore_hub/api/profile_utils.py:38` inside `get_or_create_profile()`:
+
+```python
+query = select(UserProfile).where(UserProfile.user_id == user_id)  # lookup by sub
+profile = session.execute(query).scalar_one_or_none()
+if not profile:
+    idp_name = request.headers.get(X_USER_NAME_HEADER) or None     # ONLY read
+    profile = UserProfile(user_id=user_id, name=idp_name)           # seeds display name
+```
+
+Concretely:
+
+- The header is read **only** on the `if not profile:` branch — first-time
+  profile creation.
+- It is assigned to `UserProfile.name`, a non-unique display column
+  (`src/meshcore_hub/common/models/user_profile.py:40`, no `unique=True`).
+- It is **never** a lookup key, identity check, or authorization input.
+  Identity = `X-User-Id` (`src/meshcore_hub/api/auth.py:183`);
+  authorization = `X-User-Roles` (`auth.py:210,237`,
+  `channel_visibility.py:18`).
+
+Therefore normalizing (stripping) the value cannot affect identity, auth, or
+uniqueness, and two users whose display names differ only by whitespace
+collapsing to the same string is correct display-normalization behavior, not a
+clash.
+
+### Relevant history
+
+The `X-User-Name` plumbing was introduced by the OIDC support plan
+(`20260428-1300-oidc-oauth-support`) and the consumer helper by the members
+refactor (`20260430-0805-members-refactor`). Neither sanitized the IdP-supplied
+name; this plan closes that gap.
+
+## Goals
+
+- Eliminate the `Illegal header value` 502 for any user whose IdP `name` claim
+  contains leading/trailing whitespace or embedded RFC-illegal control
+  characters (CR/LF/NUL).
+- Keep session-stored display names and seeded `UserProfile.name` values clean
+  at the ingress point.
+- Guarantee the web proxy can never construct an illegal header value,
+  regardless of future data sources (defense in depth).
+- Add regression coverage so the bug cannot silently return.
+- Trim user-supplied names at the profile-update endpoint to prevent whitespace
+  from being saved through the editor.
+
+## Non-Goals
+
+- **Backfill of existing dirty rows.** Profiles already seeded with whitespace
+  names will continue to display with whitespace until the user edits them. A
+  one-time data migration is not in scope (see Open Questions).
+- **Changing the IdP.** The IdP may legitimately emit trailing spaces; the hub
+  must tolerate this.
+- **Renaming or repurposing** the `X-User-Name` header contract.
+
+## Requirements
+
+### Functional Requirements
+
+- FR-1: A user whose IdP `name` contains leading/trailing whitespace (e.g.
+  `"Matt "`) must be able to complete login, profile bootstrap, and any
+  authenticated API call without receiving a `502`.
+- FR-2: The session-stored `name` and any newly seeded `UserProfile.name` must
+  have leading/trailing whitespace removed.
+- FR-3: The forwarded `X-User-Name` header must be a valid RFC 7230 field value
+  (no leading/trailing OWS, no embedded CR/LF/NUL).
+- FR-4: Existing authentication, authorization, identity, and uniqueness
+  semantics must be unchanged (no new collisions, no altered access control).
+
+### Technical Requirements
+
+- TR-1: Normalize in `strip_userinfo()` (`src/meshcore_hub/web/oidc.py`) so the
+  session dict never carries leading/trailing whitespace on `name`.
+- TR-2: Add a private helper in `src/meshcore_hub/web/app.py`, e.g.
+  `_sanitize_header_value(value: str) -> str`, that strips leading/trailing
+  whitespace and removes all RFC 7230-forbidden CTL characters
+  (`0x00-0x1F` excluding HTAB/SP, plus DEL `0x7F`). Apply it at both
+  header-injection sites (`app.py:751` and `app.py:1128`).
+- TR-3: Preserve `None`/empty semantics — an empty/whitespace-only name must
+  result in the `X-User-Name` header being omitted (existing
+  `if user.get("name")` guard retained).
+- TR-4: No new dependencies. No DB schema change. No migration.
+- TR-5: Tests added under `tests/test_web/` (and an OIDC unit test) following
+  existing fixture/mock patterns; full suite plus `pre-commit run --all-files`
+  must pass.
+
+## Implementation Plan
+
+### Phase 1: Ingress normalization
+
+- Edit `strip_userinfo()` in `src/meshcore_hub/web/oidc.py`: after resolving
+  `name` from the `name` / `preferred_username` / `username` / `nickname`
+  fallback chain, apply `name = name.strip() if isinstance(name, str) else name`.
+- Rationale: fixes the reported case at its source; session data and the
+  resulting seeded `UserProfile.name` are clean from the first login.
+
+### Phase 1b: Profile-update input trimming
+
+- Edit the `update_profile()` endpoint in `src/meshcore_hub/api/routes/user_profiles.py`
+  (the `PUT` handler around line 227): if the request body includes a `name`
+  field with a `str` value, apply `.strip()` before assigning it to the model.
+- Rationale: prevents users from accidentally or deliberately saving
+  whitespace-padded display names via the profile editor. Completes the
+  defense-in-depth coverage with minimal effort.
+
+### Phase 2: Header-boundary guard (defense in depth)
+
+- Add a module-level helper in `src/meshcore_hub/web/app.py`:
+  ```python
+  _ILLEGAL_HEADER_CHARS = "".join(
+      chr(c) for c in range(0x00, 0x20) if chr(c) not in "\t "
+  ) + "\x7f"
+
+  def _sanitize_header_value(value: str) -> str:
+      # RFC 7230 § 3.2.6: field-value must not contain CTL (0x00-0x1F
+      # excluding HTAB 0x09 and SP 0x20, plus DEL 0x7F), and must not
+      # have leading/trailing OWS.
+      stripped = value.strip()
+      if stripped != value:
+          logger.debug("Stripped whitespace from header value %r -> %r", value, stripped)
+      if any(c in _ILLEGAL_HEADER_CHARS for c in stripped):
+          clean = "".join(c for c in stripped if c not in _ILLEGAL_HEADER_CHARS)
+          logger.debug("Dropped control chars from header value %r -> %r", stripped, clean)
+          return clean
+      return stripped
+  ```
+- Apply at `app.py:751` (API proxy): replace
+  `headers["X-User-Name"] = user["name"]` with:
+  ```python
+  sanitized = _sanitize_header_value(user["name"])
+  if sanitized:
+      headers["X-User-Name"] = sanitized
+  ```
+- Apply at `app.py:1128` (auth-callback bootstrap): replace
+  `profile_headers["X-User-Name"] = session_user["name"]` with:
+  ```python
+  sanitized = _sanitize_header_value(session_user["name"])
+  if sanitized:
+      profile_headers["X-User-Name"] = sanitized
+  ```
+- Nested guard (`if sanitized:`) prevents emitting an empty header value when
+  the input is whitespace-only (edge case where the `user.get("name")` outer
+  guard is truthy for a whitespace-only string).
+- Debug-level logs surface malformed IdP data when `_sanitize_header_value`
+  actually alters a value, without spamming production logs.
+- Rationale: even if a future code path or an IdP quirk reintroduces dirty data
+  into the session or elsewhere, the proxy cannot emit an illegal header.
+
+### Phase 3: Tests
+
+- **Proxy regression** (`tests/test_web/`): with `session["user"]["name"]` set to
+  `"Matt "`, assert a proxied API request returns a non-502 status and that the
+  forwarded request was sent with `X-User-Name == "Matt"`. Mock
+  `request.app.state.http_client` consistent with `tests/test_web/test_app.py`.
+- **Bootstrap regression**: with a session name of `"Matt "`, assert the
+  auth-callback bootstrap request (`GET /api/v1/user/profile/me` from
+  `app.py:1128`) is forwarded with `X-User-Name == "Matt"`. Mock
+  `request.app.state.http_client` and verify the `.get()` call headers.
+- **Control char edge cases**: test names containing embedded `DEL` (`\x7f`),
+  internal `CR`/`LF`/`NUL`, and tab (which should be preserved as RFC-allowed).
+  Trailing `"\r\n"` is forwarded cleanly with control characters dropped.
+- **Whitespace-only name guard**: with `session["user"]["name"] = "   "`,
+  assert `X-User-Name` header is omitted from the forwarded request (inner
+  guard prevents empty-string header value).
+- **`strip_userinfo` unit test**: `strip_userinfo({"name": "Matt ", "sub": "x"},
+  roles_claim)` returns `{"name": "Matt", ...}`; also cover the
+  `preferred_username` and `username` fallbacks being trimmed, `None`
+  passthrough for missing names, and leading+trailing whitespace both stripped.
+- **Helper unit test**: `_sanitize_header_value("Matt \r\n") == "Matt"`;
+  `_sanitize_header_value("Ma\x7ftt") == "Matt"`; tab preserved;
+  `_sanitize_header_value("   ") == ""`.
+- **Profile-update trim** (`tests/test_api/test_user_profiles.py`):
+  `PUT /user/profile/{id}` with body `{"name": "  Matt  "}` results in
+  the profile's `name` field being stored as `"Matt"` (no leading/trailing
+  whitespace).
+
+### Phase 4: Verification
+
+- Run targeted suites, then the full suite, then pre-commit:
+  ```bash
+  source .venv/bin/activate
+  pytest --no-cov tests/test_web/ tests/test_api/test_user_profiles.py
+  pytest -nauto --no-cov
+  pre-commit run --all-files
+  ```
+- Manual smoke (optional, in the compose stack): register a test IdP user whose
+  `name` claim carries trailing whitespace and confirm login + profile update
+  succeed without a 502.
+
+## Open Questions
+
+1. **~~Profile-update trimming.~~** Resolved: included in scope (Phase 1b).
+   `PUT /user/profile/{id}` will `.strip()` the user-supplied `name`.
+2. **Backfill existing dirty rows.** Should a one-time Alembic/maintenance step
+   trim whitespace from already-seeded `UserProfile.name` values, or leave them
+   for users to self-correct via the profile editor? Default: **out of scope**.
+
+## Review
+
+**Status**: Approved with Changes
+
+**Reviewed**: 2026-07-04
+
+### Resolutions
+
+- **RFC 7230 completeness** — `_sanitize_header_value` character filter
+  expanded from `\r\n\x00` to the full set of RFC-forbidden CTL chars
+  (`0x00-0x1F` excluding HTAB/SP, plus DEL `0x7F`). Tab is preserved (RFC
+  allows it).
+- **Whitespace-only name edge case** — Both injection sites now have a nested
+  `if sanitized:` guard after sanitization, so a whitespace-only name
+  resolving to `""` correctly omits the header rather than emitting an empty
+  string value.
+- **Observability** — `_sanitize_header_value` emits debug-level logs when it
+  strips whitespace or drops control characters. Non-altering calls are silent.
+- **Bootstrap path test coverage** — Phase 3 tests now explicitly cover both
+  injection sites (`app.py:751` API proxy and `app.py:1128` auth-callback
+  bootstrap), plus control char edge cases and whitespace-only guard behavior.
+
+### Remaining Action Items
+
+- Decide on Open Question 2 (backfill) — does not gate this fix.
+
+## References
+
+- `docs/plans/20260428-1300-oidc-oauth-support/plan.md` — introduced
+  `strip_userinfo()` and the `X-User-*` proxy header contract.
+- `docs/plans/20260430-0805-members-refactor/plan.md` — introduced
+  `get_or_create_profile()`, the sole consumer of `X-User-Name`.
+- `docs/plans/20260428-1251-remove-header-auth/plan.md` — broader header-auth
+  context.
+- Key source sites:
+  - `src/meshcore_hub/web/oidc.py:58` (`strip_userinfo`)
+  - `src/meshcore_hub/web/app.py:751`, `:1128` (header injection)
+  - `src/meshcore_hub/web/app.py:793` (generic 502 handler)
+  - `src/meshcore_hub/api/profile_utils.py:38` (sole `X-User-Name` reader)
+  - `src/meshcore_hub/common/models/user_profile.py:40` (non-unique `name`)
+  - `src/meshcore_hub/api/auth.py:17` (`X_USER_NAME_HEADER` constant)
@@ -0,0 +1,127 @@
+# Tasks: Fix Illegal `X-User-Name` Header Value on New-User Registration
+
+> Generated from `plan.md` on 2026-07-04
+
+## Phase 1: Ingress Normalization
+
+- [x] Strip whitespace from IdP name claim in `strip_userinfo()`
+  - [x] In `src/meshcore_hub/web/oidc.py`, after the name fallback chain
+        (`name` / `preferred_username` / `username` / `nickname`), add
+        `name = name.strip() if isinstance(name, str) else name`
+  - [x] Verify `None` passthrough when no name claim exists
+
+## Phase 1b: Profile-Update Input Trimming
+
+- [x] Strip whitespace from user-supplied name in `update_profile()`
+  - [x] In `src/meshcore_hub/api/routes/user_profiles.py`, locate the
+        `PUT` handler body around line 227
+  - [x] Where the request body `name` is assigned to the profile model,
+        apply `.strip()` before assignment (guard with `isinstance(name, str)`)
+
+## Phase 2: Header-Boundary Guard
+
+- [x] Add `_ILLEGAL_HEADER_CHARS` constant and `_sanitize_header_value()` helper
+  - [x] Add to `src/meshcore_hub/web/app.py` (module level, near the top
+        after imports and before the first endpoint / middleware definition):
+        ```python
+        _ILLEGAL_HEADER_CHARS = "".join(
+            chr(c) for c in range(0x00, 0x20) if chr(c) not in "\t "
+        ) + "\x7f"
+
+        def _sanitize_header_value(value: str) -> str:
+            stripped = value.strip()
+            if stripped != value:
+                logger.debug("Stripped whitespace from header value %r -> %r", value, stripped)
+            if any(c in _ILLEGAL_HEADER_CHARS for c in stripped):
+                clean = "".join(c for c in stripped if c not in _ILLEGAL_HEADER_CHARS)
+                logger.debug("Dropped control chars from header value %r -> %r", stripped, clean)
+                return clean
+            return stripped
+        ```
+  - [x] Verify `logger` is already available in scope (used throughout `app.py`)
+
+- [x] Apply sanitizer at API proxy injection site (`app.py:751`)
+  - [x] Replace `headers["X-User-Name"] = user["name"]` with:
+        ```python
+        sanitized = _sanitize_header_value(user["name"])
+        if sanitized:
+            headers["X-User-Name"] = sanitized
+        ```
+  - [x] Keep the existing outer `if user.get("name"):` guard unchanged
+
+- [x] Apply sanitizer at auth-callback bootstrap injection site (`app.py:1128`)
+  - [x] Replace `profile_headers["X-User-Name"] = session_user["name"]` with:
+        ```python
+        sanitized = _sanitize_header_value(session_user["name"])
+        if sanitized:
+            profile_headers["X-User-Name"] = sanitized
+        ```
+  - [x] Keep the existing outer `if session_user.get("name"):` guard unchanged
+
+## Phase 3: Tests
+
+- [x] Add `strip_userinfo` unit tests
+  - [x] In `tests/test_web/test_oidc.py` (or create if absent): test that
+        `strip_userinfo({"name": "Matt ", "sub": "x"}, roles_claim)` returns
+        `"Matt"` for name
+  - [x] Test `preferred_username` fallback with leading/trailing whitespace
+  - [x] Test `username` fallback with whitespace
+  - [x] Test `None` passthrough when no name-like claim exists
+  - [x] Test leading+trailing whitespace both stripped
+  - [x] Follow existing test patterns (pytest fixtures, mocks)
+
+- [x] Add `_sanitize_header_value` unit tests
+  - [x] In `tests/test_web/test_app.py`: test helper directly (import from
+        `src.meshcore_hub.web.app`)
+  - [x] `_sanitize_header_value("Matt \r\n") == "Matt"` (trailing CR/LF stripped)
+  - [x] `_sanitize_header_value("Ma\x7ftt") == "Matt"` (DEL stripped)
+  - [x] Tab character `"\t"` preserved (RFC-allowed)
+  - [x] `_sanitize_header_value("   ") == ""` (whitespace-only yields empty string)
+  - [x] `_sanitize_header_value("\x00foo\x00") == "foo"` (NUL stripped)
+  - [x] `_sanitize_header_value("clean") == "clean"` (no-op passthrough)
+
+- [x] Add proxy regression test (trailing whitespace name)
+  - [x] In `tests/test_web/test_app.py`: set `session["user"]["name"] = "Matt "`
+  - [x] Mock `request.app.state.http_client` following existing patterns
+  - [x] Assert proxied request returns non-502 status
+  - [x] Assert forwarded request has `X-User-Name == "Matt"`
+
+- [x] Add bootstrap regression test
+  - [x] With session name `"Matt "`, assert the auth-callback bootstrap
+        `GET /api/v1/user/profile/me` (from `app.py:1128`) is forwarded with
+        `X-User-Name == "Matt"`
+  - [x] Mock `request.app.state.http_client` and verify `.get()` call headers
+
+- [x] Add whitespace-only name guard test
+  - [x] With `session["user"]["name"] = "   "`, assert `X-User-Name` header is
+        **omitted** from the forwarded request (inner `if sanitized:` guard)
+
+- [x] Add control char edge case tests
+  - [x] Name containing embedded DEL (`\x7f`) → forwarded cleanly
+  - [x] Name with internal CR/LF/NUL → forwarded with chars dropped
+  - [x] Name with tab (`\t`) → tab preserved in forwarded header
+
+- [x] Add profile-update trim test
+  - [x] In `tests/test_api/test_user_profiles.py`: make a `PUT` request to
+        `/user/profile/{id}` with body `{"name": "  Matt  "}`
+  - [x] Assert the profile's `name` is stored as `"Matt"` (no leading/trailing
+        whitespace)
+
+## Verification
+
+- [x] Run targeted test suites
+  - [x] `pytest --no-cov tests/test_web/ tests/test_api/test_user_profiles.py`
+  - [x] All tests pass (0 failures)
+
+- [x] Run full test suite
+  - [x] `pytest -nauto --no-cov`
+  - [x] All tests pass (0 failures)
+
+- [x] Run pre-commit checks
+  - [x] `pre-commit run --all-files`
+  - [x] All hooks pass (0 failures)
+
+- [ ] (Optional) Manual smoke test in compose stack
+  - [ ] Register a test IdP user whose `name` claim carries trailing whitespace
+  - [ ] Confirm login completes without 502
+  - [ ] Confirm profile update succeeds without 502
@@ -225,7 +225,7 @@ def update_profile(
        )

    if profile_update.name is not None:
-        profile.name = profile_update.name
+        profile.name = profile_update.name.strip()

    update_data = profile_update.model_dump(exclude_unset=True, exclude={"name"})
    for field, value in update_data.items():
@@ -38,6 +38,31 @@ PACKAGE_DIR = Path(__file__).parent
 TEMPLATES_DIR = PACKAGE_DIR / "templates"
 STATIC_DIR = PACKAGE_DIR / "static"

+# RFC 7230 §3.2.6 forbidden field-value characters: CTL (0x00-0x1F) excluding
+# HTAB (0x09) and SP (0x20), plus DEL (0x7F). These must never appear in a
+# forwarded header value or httpx rejects the request.
+_ILLEGAL_HEADER_CHARS = (
+    "".join(chr(c) for c in range(0x00, 0x20) if chr(c) not in "\t ") + "\x7f"
+)
+
+
+def _sanitize_header_value(value: str) -> str:
+    """Sanitize a header value for RFC 7230 compliance.
+
+    Strips leading/trailing OWS and removes any CTL/DEL characters that would
+    cause httpx to reject the request. Emits debug logs when altering a value.
+    """
+    stripped = value.strip()
+    if stripped != value:
+        logger.debug("Stripped whitespace from header value %r -> %r", value, stripped)
+    if any(c in _ILLEGAL_HEADER_CHARS for c in stripped):
+        clean = "".join(c for c in stripped if c not in _ILLEGAL_HEADER_CHARS)
+        logger.debug(
+            "Dropped control chars from header value %r -> %r", stripped, clean
+        )
+        return clean
+    return stripped
+

 def _load_asset_manifest() -> dict[str, Any]:
    """Load the esbuild asset manifest from dist/assets.json.
@@ -748,7 +773,9 @@ def create_app(
            if user and user.get("sub"):
                headers["X-User-Id"] = user["sub"]
                if user.get("name"):
-                    headers["X-User-Name"] = user["name"]
+                    sanitized = _sanitize_header_value(user["name"])
+                    if sanitized:
+                        headers["X-User-Name"] = sanitized
                roles = get_session_roles(request, roles_claim)
                if roles:
                    headers["X-User-Roles"] = ",".join(roles)
@@ -1125,7 +1152,9 @@ def create_app(
                "X-User-Roles": ",".join(session_user.get("roles", [])),
            }
            if session_user.get("name"):
-                profile_headers["X-User-Name"] = session_user["name"]
+                sanitized = _sanitize_header_value(session_user["name"])
+                if sanitized:
+                    profile_headers["X-User-Name"] = sanitized
            await request.app.state.http_client.get(
                "/api/v1/user/profile/me", headers=profile_headers
            )
@@ -63,6 +63,7 @@ def strip_userinfo(userinfo: dict[str, Any], roles_claim: str) -> dict[str, Any]
        or userinfo.get("username")
        or userinfo.get("nickname")
    )
+    name = name.strip() if isinstance(name, str) else name
    return {
        "sub": userinfo.get("sub"),
        "name": name,
@@ -252,6 +252,19 @@ class TestUpdateProfile:
        assert data["name"] == "New Name"
        assert data["callsign"] == sample_user_profile.callsign

+    def test_update_profile_name_trims_whitespace(
+        self, client_no_auth, sample_user_profile
+    ):
+        """Leading/trailing whitespace on a user-supplied name is stripped."""
+        response = client_no_auth.put(
+            f"/api/v1/user/profile/{sample_user_profile.id}",
+            json={"name": "  Matt  "},
+            headers=USER_HEADERS,
+        )
+        assert response.status_code == 200
+        data = response.json()
+        assert data["name"] == "Matt"
+
    def test_update_profile_callsign(self, client_no_auth, sample_user_profile):
        """Test updating profile callsign."""
        response = client_no_auth.put(
@@ -31,6 +31,10 @@ class MockHttpClient:
        # Records the params forwarded by the most recent request() call so
        # tests can assert how the proxy forwards query parameters.
        self.last_request_params: Any = None
+        # Records the headers forwarded by the most recent request() call.
+        self.last_request_headers: dict[str, Any] | None = None
+        # Records the headers forwarded by the most recent get() call.
+        self.last_get_headers: dict[str, Any] | None = None
        self._default_responses()

    def _default_responses(self) -> None:
@@ -269,6 +273,7 @@ class MockHttpClient:
    ) -> Response:
        """Mock generic request (used by API proxy)."""
        self.last_request_params = params
+        self.last_request_headers = headers
        key = f"{method.upper()}:{url}"
        if key in self._responses:
            return self._create_response(key)
@@ -277,8 +282,14 @@ class MockHttpClient:
        key = f"{method.upper()}:{base_path}"
        return self._create_response(key)

-    async def get(self, path: str, params: dict | None = None) -> Response:
+    async def get(
+        self,
+        path: str,
+        params: dict | None = None,
+        headers: dict | None = None,
+    ) -> Response:
        """Mock GET request."""
+        self.last_get_headers = headers
        # Try exact match first
        key = f"GET:{path}"
        if key in self._responses:
@@ -2,6 +2,7 @@

 import json
 from typing import Any
+from unittest.mock import AsyncMock, patch

 import pytest
 from fastapi.testclient import TestClient
@@ -11,6 +12,7 @@ from meshcore_hub.web.app import (
    _OPEN,
    _build_config_json,
    _build_endpoint_access,
+    _sanitize_header_value,
    check_api_access,
    create_app,
 )
@@ -591,3 +593,139 @@ class TestRolelessUserProfileUpdate:
            json={"callsign": "NR1"},
        )
        assert response.status_code == 200
+
+
+class TestSanitizeHeaderValue:
+    """Unit tests for the _sanitize_header_value RFC 7230 guard."""
+
+    def test_strips_trailing_whitespace(self) -> None:
+        assert _sanitize_header_value("Matt ") == "Matt"
+
+    def test_strips_leading_and_trailing_whitespace(self) -> None:
+        assert _sanitize_header_value("  Matt  ") == "Matt"
+
+    def test_strips_trailing_crlf(self) -> None:
+        assert _sanitize_header_value("Matt\r\n") == "Matt"
+
+    def test_strips_embedded_del(self) -> None:
+        assert _sanitize_header_value("Ma\x7ftt") == "Matt"
+
+    def test_strips_embedded_nul(self) -> None:
+        assert _sanitize_header_value("\x00foo\x00") == "foo"
+
+    def test_strips_embedded_cr_and_lf(self) -> None:
+        assert _sanitize_header_value("Ma\r\ntt") == "Matt"
+
+    def test_whitespace_only_yields_empty(self) -> None:
+        assert _sanitize_header_value("   ") == ""
+
+    def test_tab_preserved(self) -> None:
+        # HTAB (0x09) is allowed by RFC 7230 and must survive sanitization.
+        assert _sanitize_header_value("Ma\ttt") == "Ma\ttt"
+
+    def test_clean_value_passthrough(self) -> None:
+        assert _sanitize_header_value("clean") == "clean"
+
+    def test_strips_all_ctl_chars(self) -> None:
+        # Every CTL char 0x00-0x1F except HTAB/SP, plus DEL, is removed.
+        removed = [chr(c) for c in range(0x00, 0x20) if chr(c) not in "\t "]
+        dirty = "x" + "".join(removed) + "\x7f" + "y"
+        assert _sanitize_header_value(dirty) == "xy"
+
+
+class TestProxyHeaderSanitization:
+    """The API proxy must sanitize X-User-Name before forwarding (regression)."""
+
+    def test_trailing_whitespace_name_forwarded_clean(
+        self,
+        client_with_oidc: TestClient,
+        mock_http_client: MockHttpClient,
+    ) -> None:
+        """Reported bug: name='Matt ' caused 502; must now forward 'Matt'."""
+        dirty_user = {"sub": "user-1", "name": "Matt ", "roles": ["member"]}
+        with (
+            patch("meshcore_hub.web.app.get_session_user", return_value=dirty_user),
+            patch("meshcore_hub.web.oidc.get_session_user", return_value=dirty_user),
+        ):
+            response = client_with_oidc.get("/api/v1/nodes")
+
+        assert response.status_code != 502
+        forwarded = mock_http_client.last_request_headers
+        assert forwarded is not None
+        assert forwarded["X-User-Name"] == "Matt"
+
+    def test_whitespace_only_name_omits_header(
+        self,
+        client_with_oidc: TestClient,
+        mock_http_client: MockHttpClient,
+    ) -> None:
+        """A whitespace-only name must NOT emit an empty X-User-Name header."""
+        ws_user = {"sub": "user-1", "name": "   ", "roles": ["member"]}
+        with (
+            patch("meshcore_hub.web.app.get_session_user", return_value=ws_user),
+            patch("meshcore_hub.web.oidc.get_session_user", return_value=ws_user),
+        ):
+            response = client_with_oidc.get("/api/v1/nodes")
+
+        assert response.status_code != 502
+        forwarded = mock_http_client.last_request_headers
+        assert forwarded is not None
+        assert "X-User-Name" not in forwarded
+
+    def test_control_char_name_forwarded_clean(
+        self,
+        client_with_oidc: TestClient,
+        mock_http_client: MockHttpClient,
+    ) -> None:
+        """Embedded DEL/CR/LF in the name must be dropped before forwarding."""
+        dirty_user = {"sub": "user-1", "name": "Ma\x7f\r\ntt", "roles": ["member"]}
+        with (
+            patch("meshcore_hub.web.app.get_session_user", return_value=dirty_user),
+            patch("meshcore_hub.web.oidc.get_session_user", return_value=dirty_user),
+        ):
+            response = client_with_oidc.get("/api/v1/nodes")
+
+        assert response.status_code != 502
+        forwarded = mock_http_client.last_request_headers
+        assert forwarded is not None
+        assert forwarded["X-User-Name"] == "Matt"
+
+
+class TestBootstrapHeaderSanitization:
+    """The auth-callback bootstrap must forward a sanitized X-User-Name."""
+
+    def test_trailing_whitespace_stripped_on_bootstrap(
+        self,
+        client_with_oidc: TestClient,
+        mock_http_client: MockHttpClient,
+    ) -> None:
+        """Bootstrap GET forwards clean name after strip_userinfo trims it."""
+        token = {"userinfo": {"sub": "user-1", "name": "Matt "}}
+        with patch(
+            "meshcore_hub.web.app.oauth.oidc.authorize_access_token",
+            new_callable=AsyncMock,
+            return_value=token,
+        ):
+            client_with_oidc.get("/auth/callback", follow_redirects=False)
+
+        forwarded = mock_http_client.last_get_headers
+        assert forwarded is not None
+        assert forwarded["X-User-Name"] == "Matt"
+
+    def test_control_char_dropped_on_bootstrap(
+        self,
+        client_with_oidc: TestClient,
+        mock_http_client: MockHttpClient,
+    ) -> None:
+        """Defense-in-depth: DEL survives strip_userinfo but is removed at header."""
+        token = {"userinfo": {"sub": "user-1", "name": "Ma\x7ftt"}}
+        with patch(
+            "meshcore_hub.web.app.oauth.oidc.authorize_access_token",
+            new_callable=AsyncMock,
+            return_value=token,
+        ):
+            client_with_oidc.get("/auth/callback", follow_redirects=False)
+
+        forwarded = mock_http_client.last_get_headers
+        assert forwarded is not None
+        assert forwarded["X-User-Name"] == "Matt"
@@ -332,6 +332,36 @@ class TestStripUserinfo:
        result = strip_userinfo(userinfo, "roles")
        assert result["name"] is None

+    def test_name_trailing_whitespace_stripped(self) -> None:
+        """Test trailing whitespace is stripped from the name claim."""
+        userinfo = {"sub": "user-1", "name": "Matt "}
+        result = strip_userinfo(userinfo, "roles")
+        assert result["name"] == "Matt"
+
+    def test_name_leading_and_trailing_whitespace_stripped(self) -> None:
+        """Test both leading and trailing whitespace are stripped."""
+        userinfo = {"sub": "user-1", "name": "  Matt  "}
+        result = strip_userinfo(userinfo, "roles")
+        assert result["name"] == "Matt"
+
+    def test_preferred_username_whitespace_stripped(self) -> None:
+        """Test whitespace is stripped from the preferred_username fallback."""
+        userinfo = {"sub": "user-1", "preferred_username": "  johndoe  "}
+        result = strip_userinfo(userinfo, "roles")
+        assert result["name"] == "johndoe"
+
+    def test_username_whitespace_stripped(self) -> None:
+        """Test whitespace is stripped from the username fallback."""
+        userinfo = {"sub": "user-1", "username": "\tjohndoe\t"}
+        result = strip_userinfo(userinfo, "roles")
+        assert result["name"] == "johndoe"
+
+    def test_name_none_passthrough_not_stripped(self) -> None:
+        """Test that a missing name remains None (no AttributeError)."""
+        userinfo = {"sub": "user-1"}
+        result = strip_userinfo(userinfo, "roles")
+        assert result["name"] is None
+
    def test_roles_extracted(self) -> None:
        """Test roles are extracted from configured claim."""
        userinfo = {"sub": "user-1", "custom_roles": ["admin", "member"]}