Skip to content

feat: add extra_headers to connect() and append client=dbapi to X-Wherobots-Client (WBC-179)#71

Merged
ClayMav merged 2 commits into
mainfrom
clay/wbc-179-wherobots-python-dbapi-add-extra_headers-to-connect-and
Jul 2, 2026
Merged

feat: add extra_headers to connect() and append client=dbapi to X-Wherobots-Client (WBC-179)#71
ClayMav merged 2 commits into
mainfrom
clay/wbc-179-wherobots-python-dbapi-add-extra_headers-to-connect-and

Conversation

@ClayMav

@ClayMav ClayMav commented Jul 1, 2026

Copy link
Copy Markdown
Member

Summary

Adds an optional extra_headers param to connect() and makes the driver participate in the shared, cross-service X-Wherobots-Client HTTP header standard — the keystone for cross-service client-chain propagation.

X-Wherobots-Client is an ordered, append-only, comma-separated list of hops (client=<token>[;ver=<v>][;plat=<p>]); the leftmost is the origin and each component appends its own hop on the right (emitting , between hops). This driver appends client=dbapi;ver=<dbapi version>. The header is advisory and must never affect authentication.

What changed:

  • connect() gains extra_headers: Union[Dict[str, str], None] = None, merged into the request headers before the auth headers are applied, so Authorization/X-API-Key always win and the advisory header can never override auth.
  • The X-Wherobots-Client hop is looked up case-insensitively (HTTP header names are case-insensitive) and any differently-cased inbound key is collapsed into the canonical X-Wherobots-Client key so exactly one such header is sent. If a chain is present, , client=dbapi;ver=<version> is appended; if absent, the header is set to client=dbapi;ver=<version>.
  • The merged headers already flow to the /sql/session POST, the status-polling GET, and connect_direct() → the WebSocket upgrade (additional_headers=headers), so the appended hop is present on every hop of the connection — including the WS upgrade.
  • Backward compatible: existing/third-party direct callers that pass nothing simply get client=dbapi;ver=….
  • Version resolves via the same mechanism as gen_user_agent_header() (importlib.metadata.version("wherobots-python-dbapi"), falling back to "unknown"), factored into a shared _dbapi_version() helper.

Related Issues

Implements WBC-179.


Rubric self-assessment

  • Correctness — Appends on the right with , separator; sets the bare hop when no inbound chain; version drawn from the same source as the user-agent. Covered by tests (a) and (b) below.
  • Securityextra_headers is merged before the auth headers, so Authorization/X-API-Key always win; the advisory header never gates or influences authentication. Test test_extra_headers_do_not_override_auth proves an inbound X-Wherobots-Client cannot clobber X-API-Key.
  • Privacy — Only a static client=dbapi;ver=<v> token is appended; no PII, credentials, or caller data is added to the header.
  • Reliability — Malformed/absent inbound is tolerated: no inbound → bare hop; differently-cased inbound → matched and collapsed; empty-string inbound → treated as absent (no leading , ). No parsing of the opaque chain, so unexpected inbound content passes through untouched.
  • Scalability — O(n) over the small headers dict; no network or allocation growth.
  • Observability — This header is the observability mechanism: it makes the full client chain (origin → … → dbapi) visible to downstream services.
  • Clarity/maintainability_dbapi_version() helper is reused by gen_user_agent_header(); a WHEROBOTS_CLIENT_HEADER canonical constant and a documented _append_wherobots_client_hop() helper keep connect() readable. New param is documented in the docstring.
  • Test quality — 7 new tests in tests/test_driver.py::TestWherobotsClientHeader, all mocking requests.post/.get and the websocket connect (no external HTTP mock lib), following existing patterns:
    • test_appends_dbapi_hop_to_inbound_chain — inbound chain gets the dbapi hop appended on the right.
    • test_sets_dbapi_hop_when_no_inbound_chain — with no inbound chain, header is exactly client=dbapi;ver=<version>.
    • test_appends_case_insensitively — differently-cased inbound key is matched and collapsed to one canonical header.
    • test_forwards_arbitrary_extra_headers — arbitrary extra_headers keys are forwarded to the request.
    • test_extra_headers_do_not_override_auth — advisory header never clobbers the auth header.
    • test_ws_upgrade_carries_appended_header — the appended header also flows to the WebSocket upgrade path.
    • test_connect_direct_forwards_headers_to_wsconnect_direct forwards headers verbatim to websockets.sync.client.connect(additional_headers=...).

Visual Proof

$ pytest tests/test_driver.py
============================= test session starts ==============================
collected 19 items

tests/test_driver.py ...................                                 [100%]

============================== 19 passed in 1.53s ==============================

https://claude.ai/code/session_01QP5saxNQTUTJpMRRxZXhzR

…robots-Client (WBC-179)

Add an optional `extra_headers` param to `connect()` and make the driver
participate in the shared, cross-service `X-Wherobots-Client` header standard.

`X-Wherobots-Client` is an ordered, append-only, comma-separated list of hops
(`client=<token>[;ver=<v>][;plat=<p>]`); the leftmost is the origin and each
component appends its own hop on the right. This driver appends
`client=dbapi;ver=<dbapi version>`.

- `connect()` gains `extra_headers`, merged into the request headers before the
  auth headers are applied, so `Authorization`/`X-API-Key` always win and the
  advisory header can never affect authentication.
- The `X-Wherobots-Client` hop is appended case-insensitively (HTTP header names
  are case-insensitive) and any differently-cased inbound key is collapsed into
  the canonical `X-Wherobots-Client` key so exactly one such header is sent.
- The merged headers already flow to the `/sql/session` POST, the status-polling
  GET, and `connect_direct()` -> the WebSocket upgrade, so the appended hop is
  present on every hop of the connection.
- Absent/empty inbound chains are tolerated (bare `client=dbapi;ver=<v>`).

Factored a small `_dbapi_version()` helper shared with `gen_user_agent_header()`.

Claude-Session: https://claude.ai/code/session_01QP5saxNQTUTJpMRRxZXhzR
@ClayMav

ClayMav commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

@salty-hambot rubric

@salty-hambot salty-hambot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed by Salty Hambot 🤖🧂

Solid PR — the append-on-right, case-insensitive collapse, and auth-after-merge ordering are all correct and well-tested. One trivial double-_dbapi_version() call to tidy up and one missing test to nail down the 'extra_headers can't clobber auth' security claim; the reviewer's own self-retracted hand-wringing got left on the cutting room floor.

3 finding(s) posted.
💬 To request a re-review, comment @salty-hambot review

Comment thread wherobots/db/driver.py
Comment thread tests/test_driver.py
Comment thread tests/test_driver.py
…ariant (WBC-179)

Addresses review feedback on the X-Wherobots-Client work:

- `importlib.metadata.version` scans the installed-package database on every
  call and the version is immutable within a process, so resolve it once at
  import into `_DBAPI_VERSION: Final[str]` and reference it from both
  `gen_user_agent_header()` and the client-hop builder instead of re-resolving
  per `connect()`.
- Add two tests locking the security invariant that `extra_headers` can never
  override credentials: a hostile `X-API-Key` or `Authorization` in
  `extra_headers` is dropped because auth headers are applied after the merge,
  so the real credential always wins.

Claude-Session: https://claude.ai/code/session_01QP5saxNQTUTJpMRRxZXhzR
@ClayMav

ClayMav commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

@salty-hambot rubric

@salty-hambot salty-hambot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed by Salty Hambot 🤖🧂 — rubric mode

Verdict: ✅ pass

Dimension Verdict Notes
correctness ✅ pass Append-on-right and case-insensitive collapse iterate a snapshot before mutating; empty inbound handled as falsy.
security ✅ pass Auth headers assigned after extra_headers.update(), so Authorization/X-API-Key always win; both spoof paths now tested.
privacy ✅ pass Only a static client=dbapi;ver= token emitted; no PII or credentials added by the driver.
reliability ✅ pass No new failure modes; helper handles absent, empty, and differently-cased inbound gracefully.
scalability ✅ pass O(n) over a small fixed headers dict; module-level constant eliminates the repeated metadata scan.
observability ✅ pass The header is itself the observability artifact; no new log/metric surface needed.
clarity/maintainability ✅ pass Module constant, canonical header name, and a well-documented helper keep connect() readable.
test quality ✅ pass Nine tests cover append, no-inbound, case-collapse, forwarding, auth non-override, both spoof paths, WS upgrade, and connect_direct forwarding.

Both prior nits are buttoned up — double version call collapsed into a module-level _DBAPI_VERSION constant, and the auth-spoofing tests now lock down the security invariant for both X-API-Key and Bearer paths. Ship it.

Prior findings: ✅ 3 resolved

0 finding(s) posted.
💬 To request a re-review, comment @salty-hambot review

@ClayMav ClayMav marked this pull request as ready for review July 2, 2026 01:17
@ClayMav ClayMav requested a review from a team as a code owner July 2, 2026 01:17
@ClayMav ClayMav merged commit 109538b into main Jul 2, 2026
7 checks passed
@ClayMav ClayMav deleted the clay/wbc-179-wherobots-python-dbapi-add-extra_headers-to-connect-and branch July 2, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants