Skip to content

perps: config/init hardening — host-allowlist split, ws_base_url + password kwargs, config-vs-shorthand guard (#406, #412, #410)#416

Merged
TexasCoding merged 1 commit into
mainfrom
fix/perps-config-init-hardening-406-412-410
Jun 5, 2026
Merged

perps: config/init hardening — host-allowlist split, ws_base_url + password kwargs, config-vs-shorthand guard (#406, #412, #410)#416
TexasCoding merged 1 commit into
mainfrom
fix/perps-config-init-hardening-406-412-410

Conversation

@TexasCoding

Copy link
Copy Markdown
Owner

Cluster B of the post-review follow-ups — three config/client-construction hardenings. Pre-release polish (3.2.0 unreleased), so the new ws_base_url/password kwargs are additive with no breaking change.

#412 — host-allowlist split + WS path check (security)

PerpsConfig validated both base_url and ws_base_url against the union _PERPS_KNOWN_HOSTS, so a REST URL on a WS host (or vice versa) was accepted; and ws_base_url had no path check.

  • base_url now validates against _PERPS_REST_HOSTS, ws_base_url against _PERPS_WS_HOSTS.
  • Added a ws_base_url path check requiring /trade-api/ws/v2/margin (mirrors the REST /trade-api/v2 check).
  • Removed the now-unused _PERPS_KNOWN_HOSTS union.

#406 — client init footguns

  • (a) PerpsClient(config=…, demo=True) silently connected to whatever config specified, ignoring demo — a real-money footgun. Now combining config= with demo/base_url/ws_base_url raises.
  • (b) Added a ws_base_url kwarg to both clients + from_env (KALSHI_PERPS_WS_BASE_URL), so a REST override no longer silently leaves the WS feed on prod (a mismatch is caught by PerpsConfig's split-env guard). from_env only injects env endpoint overrides when no explicit config= is passed, so the new guard isn't tripped by env defaults.

#410 — encrypted-key passphrase

Exposed a password= kwarg on both clients + from_env for encrypted private keys, threaded to KalshiAuth.from_key_path/from_pem and try_perps_auth_from_env (the underlying signer already accepted it). from_env reads KALSHI_PERPS_PRIVATE_KEY_PASSPHRASE.

Verification

🤖 Generated with Claude Code

…ssword kwargs, config-vs-shorthand guard

Closes #412, closes #406, closes #410. Pre-release polish (3.2.0 unreleased).

#412 — PerpsConfig validated both base_url and ws_base_url against the UNION of
REST+WS hosts, so a swapped host (REST URL on a WS host, or vice versa) was
accepted; and ws_base_url had no path check. Now base_url validates against
`_PERPS_REST_HOSTS` and ws_base_url against `_PERPS_WS_HOSTS`, and ws_base_url
must carry the `/trade-api/ws/v2/margin` path (mirrors the REST path check).
Removed the now-unused `_PERPS_KNOWN_HOSTS` union.

#406 — (a) passing `config=` together with `demo=True`/`base_url`/`ws_base_url`
silently ignored the shorthand (a real-money footgun when demo=True is dropped);
now raises. (b) added a `ws_base_url` kwarg to both clients + `from_env`
(`KALSHI_PERPS_WS_BASE_URL`) so a REST override no longer leaves the WS feed on
prod — the split is caught by PerpsConfig's split-env guard if mismatched.
`from_env` only injects env endpoint overrides when no explicit `config=` is
passed, so the new guard isn't tripped by env defaults.

#410 — exposed a `password=` kwarg on both clients + `from_env` for encrypted
private keys, threaded to `KalshiAuth.from_key_path`/`from_pem` and
`try_perps_auth_from_env` (which already accepted it). `from_env` reads
`KALSHI_PERPS_PRIVATE_KEY_PASSPHRASE`.

Tests: host-swap + WS-path rejection (#412); config+shorthand guards + ws_base_url
threading (#406); encrypted-PEM password threading, sync + async (#410). mypy
strict + ruff + 1131 perps/contract tests green.
@claude

claude Bot commented Jun 5, 2026

Copy link
Copy Markdown

Code Review — perps config/init hardening (#406, #412, #410)

Overview

Three focused security/correctness hardenings for the perps client:

The motivation is solid, the code is clean, and the sync/async parity is maintained. A few things worth addressing before merge:


Issues

1. Missing from_env regression tests for the new env var and the config= skip

KALSHI_PERPS_WS_BASE_URL is documented in the docstring but has no test verifying it actually threads to client._config.ws_base_url. Similarly, the new if "config" not in kwargs: guard in from_env (which skips env endpoint injection when an explicit config= is passed) is untested. Two small tests would cover both:

def test_from_env_injects_ws_base_url(monkeypatch):
    monkeypatch.setenv("KALSHI_PERPS_WS_BASE_URL", PERPS_DEMO_WS_URL)
    monkeypatch.setenv("KALSHI_PERPS_API_BASE_URL", PERPS_DEMO_BASE_URL)
    client = PerpsClient.from_env()
    assert client._config.ws_base_url == PERPS_DEMO_WS_URL
    client.close()

def test_from_env_config_kwarg_skips_env_injection(monkeypatch):
    monkeypatch.setenv("KALSHI_PERPS_DEMO", "true")
    # config= is given explicitly; env DEMO should not also be injected (would hit the guard)
    client = PerpsClient.from_env(config=PerpsConfig.production())
    assert client._config.base_url == PERPS_PRODUCTION_BASE_URL
    client.close()

2. Error message prints None for the unconstrained field

The conflict error formats both sides unconditionally:

f"base_url={base_url!r} / ws_base_url={ws_base_url!r} pointing elsewhere"

When only one is conflicting (e.g., demo=True, ws_base_url="wss://custom.host/..." with no base_url), the message reads base_url=None / ws_base_url='wss://...' pointing elsewhere. The None reads as noise rather than information. Consider filtering to only the non-None values:

parts = []
if base_url is not None and base_url.rstrip("/") != PERPS_DEMO_BASE_URL:
    parts.append(f"base_url={base_url!r}")
if ws_base_url is not None and ws_base_url.rstrip("/") != PERPS_DEMO_WS_URL:
    parts.append(f"ws_base_url={ws_base_url!r}")
conflict = " / ".join(parts)
raise ValueError(
    f"Conflicting environment: demo=True together with {conflict} pointing "
    "elsewhere. ..."
)

3. if ws_base_url: truthy check vs is not None (minor)

Injection into config_kwargs uses a truthy check (if ws_base_url:), while the conflict detection uses is not None. This matches the existing base_url pattern, so it's consistent, but it creates an invisible edge case: ws_base_url="" escapes injection but triggers the conflict check under demo=True. Not a real-world scenario, but worth aligning for correctness parity — if ws_base_url is not None: in both spots would be more precise.


Positives

  • Host-allowlist split in _validate_perps_url is the right fix — accepting a REST host for ws_base_url was a genuine silent footgun.
  • WS path check is a clean mirror of the existing REST path check; the test (test_ws_base_url_rejects_wrong_path) correctly verifies it fires even with the conftest's KALSHI_PERPS_ALLOW_UNKNOWN_HOST=1 because the target host is known.
  • config= + shorthand guard error message is clear and actionable.
  • Encrypted-key tests (TestIssue410EncryptedKeyPassword) are non-vacuous — they actually load an encrypted PEM, so they'd fail without the fix.
  • from_env env-injection guard (if "config" not in kwargs:) correctly avoids tripping the new __init__ guard when a caller passes config= to from_env.
  • asyncio_mode = "auto" in pyproject.toml means the bare async def test_password_threaded_async is picked up correctly without an explicit decorator.
  • _encrypted_pem helper is a clean, reusable test utility.

Summary

The logic is correct and the intent is well-motivated. The main gap is test coverage for the two new from_env behaviors (env var threading + config= skip). The error-message None issue is cosmetic but will confuse users hitting the guard with only one of the two endpoint kwargs set. Both are quick fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment