stacklok / toolhive / 28452352865

68%

Build Type

push

github

Committed by web-flow

Commit Message

Stop fabricating partial identity in RestoreSession (#5650)

* Stop fabricating partial identity in RestoreSession

RestoreSession constructed &auth.Identity{Subject, Claims} with Token
and UpstreamTokens unset, then passed it to makeBaseSession where it
propagated to the identityRoundTripper fallback in each backend
connector. This violated the field-completeness contract on *auth.Identity
and was a latent bug for any future consumer (audit log, outgoing-auth
strategy) that assumes a non-nil *Identity is fully populated.

Implements changes for issue #5336:
- Remove partial identity construction from RestoreSession; pass nil to
  makeBaseSession instead. Binding validation (binding.Parse) is kept to
  catch corrupted stored bindings before any backend work is attempted.
- Add field-completeness contract to auth.Identity godoc: a non-nil
  *Identity is always complete; nil represents the anonymous / no-identity
  state, not a partially-initialised struct.
- Replace the test that pinned the old partial-identity behavior with one
  that asserts nil is passed to the backend connector on restore.

Live tool calls are unaffected: TokenValidator.Middleware places a
fully-populated identity on req.Context() before requests reach the
backend connectors, and identityRoundTripper already only injects its
captured fallback when the request context carries none (see #5323).

* Address review feedback: fix stale comment, add E2E TODO

- Update RestoreSession implementation doc comment to reflect that
  connectors now receive nil identity (the old comment said identity was
  reconstructed from the stored iss/sub binding, which is no longer true)
- Add TODO comment to the cross-pod restore E2E test marking the
  upstream-auth extension from AC4 of issue #5336 as dependency-gated
  on a live OIDC provider in the test environment

* Rename stale test to reflect current behavior

TestRestoreSession_PopulatesBothSubjectFieldAndClaims described behavior
removed by the pa... (continued)

Coverage Stats

28 of 28 new or added lines in 5 files covered. (100.0%)

20 existing lines in 5 files now uncovered.

70949 of 105123 relevant lines covered (67.49%)

63.35 hits per line