Skip to content

feat: add TLS verification option (verify) to Configuration/PlaneClient/OAuthClient - #69

Open
pesnik wants to merge 1 commit into
makeplane:mainfrom
pesnik:ssl-verify
Open

feat: add TLS verification option (verify) to Configuration/PlaneClient/OAuthClient#69
pesnik wants to merge 1 commit into
makeplane:mainfrom
pesnik:ssl-verify

Conversation

@pesnik

@pesnik pesnik commented Aug 25, 2026

Copy link
Copy Markdown

Summary

Adds a verify: bool | str = True option, mirroring requests.Session.verify semantics exactly, so self-hosted Plane instances behind an internal CA or a certificate that doesn't match the hostname it's served on can be reached. Today there is no TLS configuration anywhere in the SDK — this fills that gap.

  • Configuration(..., verify=True) — new field, default unchanged.
  • PlaneClient(..., verify=True) — passed straight through to Configuration.
  • OAuthClient(..., verify=True) — same option on its own requests.Session (separate HTTP client, not backed by Configuration).
  • BaseResource.__init__ sets self.session.verify = config.verify. Every resource (and sub-resource — e.g. work_items.comments, which builds its own BaseResource off the same Configuration) gets it from this single point.

verify accepts the same three states requests does:

  • True (default) — verify against the system CA store, unchanged behavior.
  • False — disable verification entirely.
  • a path string — treated as a CA bundle for a custom/internal CA.

Real-world motivation: a self-hosted Plane instance behind an ingress that serves a wildcard cert (*.company.com) not valid for a two-label subdomain (plane.team.company.com) — a hostname-mismatch failure, not an untrusted-chain one. No existing knob (REQUESTS_CA_BUNDLE, SSL_CERT_FILE, etc.) fixes hostname verification, only the trust anchor. Related: #5581 hits the same class of problem from Plane's own outbound OAuth calls (self-signed cert on a self-hosted GitLab instance) — different code path, but the same gap: no self-hosted deployment in this ecosystem currently has a way to configure TLS trust. Prior art for this exact shape: the Kubernetes Python client's Configuration.verify_ssl / ssl_ca_cert.

Test plan

  • New unit tests in tests/unit/test_config.py (pure construction, no network): default True, False propagates to Configuration and to every resource's Session.verify (including a sub-resource), a CA-bundle path propagates verbatim, same three checks for OAuthClient.
  • ruff check clean on all changed files.
  • Manually verified end-to-end against a real self-hosted instance exhibiting exactly this hostname-mismatch failure: verify=False turns a SSLCertVerificationError into a normal authenticated API response.
  • Default behavior unchanged — verify=True unless explicitly set.

Summary by CodeRabbit

  • New Features

    • Added configurable TLS certificate verification for SDK clients.
    • TLS verification can remain enabled, be disabled, or use a custom CA certificate bundle.
    • The setting is consistently applied across standard, sub-resource, and OAuth client connections.
  • Documentation

    • Updated SDK configuration documentation with the new TLS verification option.

…ent, OAuthClient

Adds a verify: bool | str = True field mirroring requests.Session.verify
semantics, so self-hosted Plane instances with an internal CA or a
misconfigured/mismatched certificate can be reached. Threaded through
Configuration -> BaseResource.session.verify (covers all resources and
sub-resources, which each construct their own Session off the same
Configuration) and separately through OAuthClient's own Session.

Default behavior is unchanged (verify=True).
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ea028622-e04b-497e-b76f-2f7bffacf08a

📥 Commits

Reviewing files that changed from the base of the PR and between 31a4f9e and 3a107a3.

📒 Files selected for processing (6)
  • README.md
  • plane/api/base_resource.py
  • plane/client/oauth_client.py
  • plane/client/plane_client.py
  • plane/config.py
  • tests/unit/test_config.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The SDK adds a verify option to configuration, PlaneClient, and OAuthClient. The option accepts a boolean or CA bundle path and propagates to HTTP session TLS verification. Documentation and unit tests cover defaults and propagation.

Changes

TLS verification configuration

Layer / File(s) Summary
Configuration propagation
plane/config.py, plane/client/plane_client.py
Configuration stores verify, and PlaneClient forwards the option to it.
Session verification application
plane/api/base_resource.py, plane/client/oauth_client.py
API resource sessions and OAuth sessions apply the configured boolean or CA bundle path to requests.Session.verify.
Validation and documentation
README.md, tests/unit/test_config.py
The configuration table documents verify. Unit tests cover defaults, disabled verification, CA bundle paths, and propagation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 3a107

The PR adds configurable TLS verification while keeping secure verification enabled by default; no actionable merge-blocking risk remains based on the supplied evidence.

Suggested reviewers: dheeru0198

Sequence Diagram(s)

sequenceDiagram
  participant PlaneClient
  participant Configuration
  participant BaseResource
  participant RequestsSession
  PlaneClient->>Configuration: forward verify
  Configuration->>BaseResource: provide config.verify
  BaseResource->>RequestsSession: set session.verify
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 5 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding the TLS verification option to Configuration, PlaneClient, and OAuthClient.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 5 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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