Skip to content

fix(sandbox): verify skipTLSVerify/caFile kubeconfig behavior under Bun (#1849)#1916

Open
larryro wants to merge 1 commit into
mainfrom
tale/sx72ebzew1j57jmc9kp1w9nw6x8937ke
Open

fix(sandbox): verify skipTLSVerify/caFile kubeconfig behavior under Bun (#1849)#1916
larryro wants to merge 1 commit into
mainfrom
tale/sx72ebzew1j57jmc9kp1w9nw6x8937ke

Conversation

@larryro

@larryro larryro commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Resolves #1849. Empirically verifies that @kubernetes/client-node@1.4.0 + Bun 1.3.14 silently ignores kubeconfig TLS knobs, then removes the misleading dead code and documents the real TLS contract.

Test setup: Bun test server with self-signed cert + node-fetch v2 + https.Agent. Results:

  • https.Agent({ rejectUnauthorized: false }) (from skipTLSVerify: true) → INERT: TLS still fails
  • https.Agent({ ca: certData }) (from caFile) → INERT: CA not applied
  • NODE_EXTRA_CA_CERTS=/path/to/ca.crtworks

Root cause: node-fetch@2 calls node:https.request passing the https.Agent via the agent option. Bun's node:https shim stores the agent options but does not apply rejectUnauthorized or ca to the actual TLS connection.

Changes

  • k8s-client.ts: Remove skipTLSVerify: true from the bearer-token kubeconfig (it's inert under Bun and looks like a security bypass without being one). Update the file-level comment from "AUTH NOTE" to "TLS NOTE" enumerating all three inert kubeconfig knobs (caFile, skipTLSVerify, client certs).
  • k8s-client.test.ts: Add makeK8sClient kubeconfig shape test group pinning that skipTLSVerify is never set and caFile is conditional on SANDBOX_K8S_CAFILE.
  • docs/kubernetes.md: Add "Bun TLS contract" section with a findings table and update the NODE_EXTRA_CA_CERTS env row to call it out as the only working CA-trust mechanism.

Test plan

  • bun test in services/sandbox — 329 tests, all pass
  • bun run typecheck in services/sandbox — clean
  • Review: skipTLSVerify no longer appears in the cluster config produced by makeK8sClient

…un (#1849)

Empirical testing under Bun 1.3.14 confirms that @kubernetes/client-node@1.4.0
sends requests via node-fetch@2 -> node:https.request with an https.Agent, and
that Bun's node:https shim silently ignores the agent's ca and
rejectUnauthorized fields at the TLS layer.

Consequences:
- caFile / caData in the kubeconfig: INERT under Bun (CA not loaded)
- skipTLSVerify: true: INERT under Bun (TLS still verified, NOT bypassed)
- NODE_EXTRA_CA_CERTS: the only working CA-trust mechanism under Bun

Changes:
- Remove skipTLSVerify: true from makeK8sClient (dead code that misleadingly
  looks like a security bypass but does nothing under Bun)
- Update the file-level TLS NOTE to enumerate all three inert knobs
- Add tests pinning the kubeconfig shape (skipTLSVerify absent, caFile
  conditional on SANDBOX_K8S_CAFILE, bearer token on user entry)
- Add docs/kubernetes.md Bun TLS contract section with findings table
  and updated NODE_EXTRA_CA_CERTS env table row
@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@larryro, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 9 minutes and 13 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6f8f5678-aac2-4953-8c39-49a35833a1f6

📥 Commits

Reviewing files that changed from the base of the PR and between 5946d86 and bae3cee.

📒 Files selected for processing (3)
  • services/sandbox/docs/kubernetes.md
  • services/sandbox/src/backend/kubernetes/k8s-client.test.ts
  • services/sandbox/src/backend/kubernetes/k8s-client.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tale/sx72ebzew1j57jmc9kp1w9nw6x8937ke

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


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 and usage tips.

@larryro

larryro commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

⚠️ Conflicting PRs for #1849 — do NOT merge any of these until resolved.

Three open PRs address the same question, and two reach opposite conclusions about whether Bun honors kubeconfig TLS knobs:

This is security-critical: merging the wrong conclusion could leave TLS verification silently disabled in production, or remove code that was actually doing the verification. This cannot be resolved by review alone — it needs an empirical test under the real Bun version (issue a request with a kubeconfig carrying caFile/skipTLSVerify and observe whether TLS is actually enforced/skipped). Holding all three for a human decision; only the PR matching the verified behavior should be merged, the others closed.

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.

sandbox/k8s: verify skipTLSVerify/caFile behavior of kubeconfig under Bun

1 participant