Skip to content

fix: harden wrappers, close review findings and add optional CA verif… - #14

Closed
sgofferj wants to merge 1 commit into
mainfrom
feat/review-fixes
Closed

sgofferj wants to merge 1 commit into
mainfrom
feat/review-fixes

Conversation

@sgofferj

Copy link
Copy Markdown
Owner

…ication

Review fixes:

  • build_predicate_feed: archive now defaults to False (archived feeds bloat Postgres); live fixture pins archive=False explicitly
  • user_exists/group_exists return False on failed requests instead of crashing on error text; group readback helpers raise a clear ValueError
  • mission API: URL-encode query parameters (create_mission, subscriptions, role, content ops) via the shared _query helper
  • ConnectionHelper.request raises ValueError for unknown HTTP methods
  • download_certificate uses the class _unwrap like its siblings
  • dedupe input-naming rule in submission module docstring

CA verification:

  • Server()/ConnectionHelper accept ca_cert: when given, the server certificate is verified against that CA with hostname checking enabled, while still presenting the client certificate for mTLS; without it, behaviour is unchanged (CERT_NONE, documented in README)
  • SSL context creation extracted into testable create_client_ssl_context()
  • document deliberate CERT_NONE default in README
  • tests: real in-loop TLS handshakes for pinned/foreign CA, constructor pass-through, previously untested methods (226 tests, 98% cov)

…ication

Review fixes:
- build_predicate_feed: archive now defaults to False (archived feeds
  bloat Postgres); live fixture pins archive=False explicitly
- user_exists/group_exists return False on failed requests instead of
  crashing on error text; group readback helpers raise a clear ValueError
- mission API: URL-encode query parameters (create_mission, subscriptions,
  role, content ops) via the shared _query helper
- ConnectionHelper.request raises ValueError for unknown HTTP methods
- download_certificate uses the class _unwrap like its siblings
- dedupe input-naming rule in submission module docstring

CA verification:
- Server()/ConnectionHelper accept ca_cert: when given, the server
  certificate is verified against that CA with hostname checking enabled,
  while still presenting the client certificate for mTLS; without it,
  behaviour is unchanged (CERT_NONE, documented in README)
- SSL context creation extracted into testable create_client_ssl_context()
- document deliberate CERT_NONE default in README
- tests: real in-loop TLS handshakes for pinned/foreign CA, constructor
  pass-through, previously untested methods (226 tests, 98% cov)
@sgofferj sgofferj closed this Aug 26, 2026
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.

1 participant