Skip to content

fix: secure WebSocket authentication and diagnostic boundaries - #837

Merged
AlexanderZ-Band merged 10 commits into
mainfrom
feat/security-use-header-only-websocket-auth-in-phoenix-INT-1838
Oct 20, 2026
Merged

AlexanderZ-Band merged 10 commits into
mainfrom
feat/security-use-header-only-websocket-auth-in-phoenix-INT-1838

Conversation

@AlexanderZ-Band

@AlexanderZ-Band AlexanderZ-Band commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Why

The SDK's WebSocket error probe and retained refusal fields can expose credentials after the connection attempt.

What

Adopt Phoenix header-only authentication (phoenix-channels-python-client>=0.3.1,<0.4, locked from the registry), reuse its safe connection logger for probes and failures, redact retained error fields, and encode agent identity as one query value. Keep the existing test layout, add focused wire/log regressions, and update the fast-reconnect test fixture for Phoenix's Pydantic ReconnectPolicy.

Scope

Python WebSocket transport only, coordinated by INT-1838. Phoenix 0.3.1 is published, so the lock is real. Moving to Phoenix 0.4.0 (reconnect default, terminal 403) is INT-1859. Consumer manifests and deployment work are outside this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T5N5cEQQgnb9GXfNTcBSve

@linear-code

linear-code Bot commented Oct 18, 2026

Copy link
Copy Markdown

INT-1838

@AlexanderZ-Band
AlexanderZ-Band marked this pull request as ready for review October 18, 2026 05:35
@AlexanderZ-Band
AlexanderZ-Band requested review from a team and amit-gazal-band October 18, 2026 05:35

@AlexanderZ-Band AlexanderZ-Band left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my-code-review --fix: converged in cycle 1. Independent A–F passes found no verified in-scope issues or cleanup worth changing; no existing feedback required handling. Scope: Python WebSocket dependency adoption, connection/probe logging and retained diagnostics.

Validation against updated local Phoenix source ada7555: 77 focused tests passed; repository-wide Ruff, format and pyrefly checks passed. The broad 8,690-test suite was intentionally interrupted to keep verification proportional and is not a pass.

Release gate remains: exact-head CI and a fresh uv lock --check fail because the required Phoenix >=0.3.1 release is not published (registry currently offers <=0.2.5). Publish the dependency, refresh the real lock, and rerun CI before merge. No source changes, merges or releases performed.

@AlexanderZ-Band

AlexanderZ-Band commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator Author

Checkpoint — 2026-10-20

Merged to main as 0c94e9908 (10:21 UTC). Not yet released or adopted. Owner: Alexander Zaikman. Tracking: INT-1838, parent INT-1807.

Merged head 8bc9edd24 had full CI green: 40 checks passed, none failed, including the test, test-crewai, test-parlant and test-strands-nooa matrices on ubuntu and windows, Python 3.11–3.13. The full local unit suite also passed on Python 3.12 (9,088 passed, 157 skipped). main pins phoenix-channels-python-client>=0.3.1,<0.4 with the real 0.3.1 lock.

GitHub still showed REVIEW_REQUIRED when the merge queue accepted it; no admin bypass was used.

Continue here

  1. Release PR #873 bumps 6.0.0 to 6.0.1 and ships this fix together with fix: reconnect after WebSocket close 1000/1001 #869. An explicit command naming chore: release main #873 is needed before merge.
  2. After that merge, verify band-publish.yml, the PyPI artifact and a released-wheel install, then run uv run --locked pytest tests/websocket/test_client.py tests/websocket/test_security.py -q --no-cov.
  3. Record the actual SDK version and adoption in INT-1807.

Follow-ups: INT-1859 and INT-1861. Docker kit and example lockfiles still pin older Phoenix and are excluded.

xxxxxxxxxxxxx and others added 2 commits October 19, 2026 19:03
The pin moved to >=0.3.1,<0.4 without a lock refresh, so `uv sync --locked`
and the lock-sync lint step failed on every job. 0.3.1 was published after
the last CI run, which is why resolution failed at the time.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MaoYFzCV2bJRD4sDDVD7td
@AlexanderZ-Band
AlexanderZ-Band added this pull request to the merge queue Oct 19, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 19, 2026
@AlexanderZ-Band
AlexanderZ-Band marked this pull request as draft October 20, 2026 08:17
AlexanderZ-Band and others added 3 commits October 20, 2026 11:18
…-in-phoenix-INT-1838

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5N5cEQQgnb9GXfNTcBSve
Signed-off-by: Alexander Zaikman <alexander.zaikman@band.ai>
…ect fixture

Phoenix 0.3 made ReconnectPolicy a frozen Pydantic model, so
dataclasses.replace raised TypeError in millisecond_reconnects and errored
every tests/platform/linkclose test.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5N5cEQQgnb9GXfNTcBSve
Signed-off-by: Alexander Zaikman <alexander.zaikman@band.ai>
@AlexanderZ-Band
AlexanderZ-Band marked this pull request as ready for review October 20, 2026 09:23
@AlexanderZ-Band
AlexanderZ-Band added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 0c94e99 Oct 20, 2026
53 checks passed
@AlexanderZ-Band
AlexanderZ-Band deleted the feat/security-use-header-only-websocket-auth-in-phoenix-INT-1838 branch October 20, 2026 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants