feat: add TLS verification option (verify) to Configuration/PlaneClient/OAuthClient - #69
feat: add TLS verification option (verify) to Configuration/PlaneClient/OAuthClient#69pesnik wants to merge 1 commit into
Conversation
…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).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe SDK adds a ChangesTLS verification configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to 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: 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
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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)
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. Comment |
Summary
Adds a
verify: bool | str = Trueoption, mirroringrequests.Session.verifysemantics 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 toConfiguration.OAuthClient(..., verify=True)— same option on its ownrequests.Session(separate HTTP client, not backed byConfiguration).BaseResource.__init__setsself.session.verify = config.verify. Every resource (and sub-resource — e.g.work_items.comments, which builds its ownBaseResourceoff the sameConfiguration) gets it from this single point.verifyaccepts the same three statesrequestsdoes:True(default) — verify against the system CA store, unchanged behavior.False— disable verification entirely.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'sConfiguration.verify_ssl/ssl_ca_cert.Test plan
tests/unit/test_config.py(pure construction, no network): defaultTrue,Falsepropagates toConfigurationand to every resource'sSession.verify(including a sub-resource), a CA-bundle path propagates verbatim, same three checks forOAuthClient.ruff checkclean on all changed files.verify=Falseturns aSSLCertVerificationErrorinto a normal authenticated API response.verify=Trueunless explicitly set.Summary by CodeRabbit
New Features
Documentation