Harden Kind lab ownership and upgrade safety

[Timpan4]

Follow-up to #272.

• reject symlinked ownership and kubeconfig paths before reads or Kind operations
• require recorded disableDefaultCNI proof before reusing a persistent dev lab
• keep explicit dev:kind:down compatible with legacy ownership records
• assert exact production-shaped Argo and Helm mock fixture names

Checks:

• bun test e2e/harness/harness.test.ts tests/tauri-wrappers.test.ts
• bun run typecheck

[coderabbitai]

Warning

Review limit reached

@Timpan4, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

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 reviews.

How do review limits work?

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

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, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 73d19b94-b1a2-4db3-bd01-b0e55db52d06

📥 Commits

Reviewing files that changed from the base of the PR and between d6e5567 and 92cc938.

📒 Files selected for processing (4)

• e2e/harness/harness.test.ts
• e2e/harness/ownership.ts
• e2e/lifecycle.ts
• tests/tauri-wrappers.test.ts

📝 Walkthrough

Walkthrough

Ownership records now include default-CNI proof and realpath/symlink validation. Lifecycle operations enforce these checks, dev teardown can relax CNI validation, and tests add filesystem safety and exact GitOps/Helm assertions.

Changes

Ownership validation and lifecycle integration

Layer / File(s)	Summary
Ownership contract and filesystem checks e2e/harness/ownership.ts	Ownership records include disableDefaultCNI; on-disk validation rejects symlinked paths and paths outside the resolved ownership directory.
Lifecycle ownership enforcement e2e/lifecycle.ts	Lifecycle operations validate ownership on disk, creation records provider and CNI state, and dev-down disables the default-CNI requirement.
Ownership regression coverage e2e/harness/harness.test.ts	Tests cover symlinked kubeconfig rejection and legacy dev records with optional cleanup permission.

Browser mock assertion precision

Layer / File(s)	Summary
GitOps and Helm result assertions tests/tauri-wrappers.test.ts	Argo application and Helm release checks now assert exact ordered name lists.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Lifecycle
  participant Ownership
  participant Filesystem
  Lifecycle->>Ownership: Load or create ownership record
  Ownership->>Filesystem: Check real paths and symlink components
  Filesystem-->>Ownership: Return validation result
  Ownership-->>Lifecycle: Return validated ownership
  Lifecycle->>Filesystem: Perform cluster operation

Loading

Possibly related PRs

• Timpan4/kubecove#265: Both changes update E2E lifecycle ownership-record enforcement.

Poem

A rabbit checks each path with care,
No sneaky links may wander there.
CNI proof joins the record’s song,
Dev teardown knows when rules belong.
Argo and Helm names hop just right.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title clearly summarizes the main change: hardening Kind lab ownership and upgrade safety.
Description check	✅ Passed	The description matches the changeset and accurately mentions symlink checks, default-CNI validation, legacy teardown compatibility, and fixture assertions.

---

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.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

e2e/lifecycle.ts (1)

289-292: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Bring this module below the TypeScript soft cap.

The file reaches Line 334, exceeding the 300-line soft cap. Extract a cohesive lifecycle concern (for example diagnostics or ownership loading/cleanup) rather than extending this module further.

As per coding guidelines, “Keep files within the documented size caps: … TS 300 soft/600 hard.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/lifecycle.ts` around lines 289 - 292, The lifecycle module exceeds the
300-line TypeScript soft cap. Extract a cohesive concern from the code around
cluster creation and cleanup—such as diagnostics or ownership
loading/cleanup—into a focused module, then update the lifecycle flow to use the
extracted symbols while preserving existing behavior of the create, bootstrap,
diagnostics, and removeCluster paths.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e2e/harness/harness.test.ts`:
- Around line 44-53: Make the “refuses symlinked ownership paths” test
platform-safe by skipping or gating its symlink setup on Windows, avoiding an
EPERM failure before assertOwnedOnDisk runs. Preserve the existing symlink
assertion and cleanup behavior on supported platforms.

In `@e2e/lifecycle.ts`:
- Line 130: Ensure ownership-path validation occurs before filesystem access at
both sites: in readOwnership, validate the ownership directory and
ownership.json path before readFile, while preserving the subsequent ownership
record validation; in the flow around existsSync(record.kubeconfig), call
assertOwnedOnDisk before checking the file’s existence.

---

Nitpick comments:
In `@e2e/lifecycle.ts`:
- Around line 289-292: The lifecycle module exceeds the 300-line TypeScript soft
cap. Extract a cohesive concern from the code around cluster creation and
cleanup—such as diagnostics or ownership loading/cleanup—into a focused module,
then update the lifecycle flow to use the extracted symbols while preserving
existing behavior of the create, bootstrap, diagnostics, and removeCluster
paths.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ddf8f9ab-8698-4304-8922-98e104971920

📥 Commits

Reviewing files that changed from the base of the PR and between 5b108c5 and 2a98dc9.

📒 Files selected for processing (4)

• e2e/harness/harness.test.ts
• e2e/harness/ownership.ts
• e2e/lifecycle.ts
• tests/tauri-wrappers.test.ts

[codspeed]

Merging this PR will not alter performance

✅ 36 untouched benchmarks
⏩ 4 skipped benchmarks¹

---

_{Comparing fix/kind-lab-ownership (92cc938) with main (839db18)}

Footnotes

1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩