fix: drop the connect token once not needed

[kerthcet]

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #

Special notes for your reviewer

Does this PR introduce a user-facing change?

Summary by CodeRabbit

• Changes
  • Pods that declare at least one container port receive a connect URL and token when provisioned. Portless Pods are still reserved, but no connect URL or token is returned.
  • Credential creation for an existing sandbox is no longer available through the client interface. Connect credentials are minted during sandbox creation and cannot be retrieved afterward.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5de3c5da-a312-401b-92a1-905ee22772a2

---

📥 Commits

Reviewing files that changed from the base of the PR and between 0574adf and 6e6bbf7.

---

📒 Files selected for processing (3)

• pkg/provider/modal/client.go
• pkg/provider/modal/modal.go
• pkg/provider/modal/modal_test.go

---

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

---

---

📝 Walkthrough

Walkthrough

The Modal client no longer exposes credential minting for existing sandboxes. Provisioning continues to reserve portless Pods, but returns connect credentials only when a Pod declares a container port.

Changes

Modal connect credential handling

Layer / File(s)	Summary
One-shot credential contract pkg/provider/modal/client.go, pkg/provider/modal/modal.go	The client contract and implementation remove MintConnectCredential. CreateSandbox documentation describes one-shot credential minting. A TODO notes that minting currently serves as the placement wait.
Port-gated provisioning result pkg/provider/modal/modal.go, pkg/provider/modal/modal_test.go	Provisioning copies the credential URL and token into the result only when the Pod declares a port. Tests cover ported and portless Pods, and the fake client no longer implements credential minting.

Priority: ⬇️ Low

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

Change: Bug fix

---

Merge Risk | ⚪ Minimal · up to 6e6bb

Merge Risk: ⚪ Minimal · up to 6e6bb

Portless Pods remain reserved without storing connect credentials. No actionable merge-blocking risk is established; merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6e6bb

The change narrows credential distribution rather than expanding access. No introduced security flaw was established, but credentials are still minted for workloads without declared ports, and their remote lifetime and enforcement remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The supported credential flow concerns an individual created sandbox and its Pod-namespaced Secret. Suppressing portless results reduces distribution through this flow. The evidence does not establish the maximum server-side authority of a token, cross-tenant isolation, or effective access to the Secret.

Security Findings and Attack Paths

• observed — The supplied credential candidate is deferred, not a verified vulnerability. Local source confirms continued minting followed by reduced publication, but does not resolve the candidate's SDK and sandbox-port proof gap. No introduced or worsened attack path was established from the inspected comparison.

Trust Boundaries and Controls

• observed — Before provisioning, the handler resolves pool policy and placement identity and stops when either cannot be established. Returned bearer tokens travel through the separate Secret write rather than Pod metadata. Namespacing and UID ownership are visible controls, but neither proves effective Secret-read authorization or remote token enforcement.

---

Pre-merge checks | 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title accurately describes the change to stop returning connect tokens when they are not needed. It is concise and related to the conditional credential behavior.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

---

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

[kerthcet]

/lgtm
/kind bug

[InftyAI-Agent]

Approved: PR has both lgtm and approved labels

[InftyAI-Agent]

Approved: PR has both lgtm and approved labels