fix(agentbox): self-heal runtime/agentbox mTLS cert mismatch via CA fingerprint - #352
Conversation
When the runtime CA changes (e.g. the siclaw-runtime-ca Secret is regenerated by a render-then-apply or a lookup-less helm op), every already-running AgentBox pod keeps a cert signed by the old CA and a mounted ca.crt trusting the old CA. The runtime then rejects it (403 / handshake failure) in both directions, with no recovery — the pod stays Running but unusable until manually deleted. Stamp each pod and its -cert Secret with a `<prefix>/ca-fp` label = a fingerprint of the signing CA. The runtime reuses a running pod only when that fingerprint matches its current CA; a mismatch (or a legacy pod with no label) is treated as stale and the pod is deleted + respawned with a fresh cert. This makes any CA change self-healing instead of a stuck mismatch — a no-op when the CA is unchanged. - cert-manager: add caFingerprint() (sha256 of CA PEM, 16 hex chars) - k8s-spawner: stamp pod+secret label; recycle stale pod in the reuse branch; expose caFingerprint(); return it from get() - manager: isCertFresh() gate on the getOrCreateK8s reuse path - AgentBoxInfo.caFingerprint + optional BoxSpawner.caFingerprint() Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
LikiosSedo
left a comment
There was a problem hiding this comment.
Reviewed the full diff plus the secret-creation and reuse paths. This is correct and well-targeted — pure label comparison, no TLS probe, a genuine no-op when the CA is unchanged, and it turns a permanently-stuck 403 into one transparent respawn.
I specifically verified the heal is complete end to end: on the stale path the pod is deleted, and the -cert Secret create hits the existing 409 → delete + recreate branch, so the new pod mounts a cert + ca.crt signed by the current CA. mTLS recovers in both directions in a single respawn — no "pod recreated but still serving the old cert" gap. Moving the if (!certManager) throw to the top of spawn() is correct now that the reuse branch needs caFp. Back-compat (no-CA spawner → treated fresh; unlabeled legacy pod → recycled once) and the two-layer check (manager.isCertFresh + spawner reuse branch) are consistent. LGTM.
One low-priority note inline.
Merge coordination: this PR and #350 both edit the same lines of docs/design/invariants.md §1.3 and conflict with each other — verified via git merge-tree. Each merges cleanly into main alone; the second to land needs to hand-merge that section: keep #350's idle-window paragraph + this PR's CA-fingerprint bullet + a combined Source line.
| - The `global/`, `skillset/`, and `user/` skill subdirectories in a pod are managed by resource sync — wiped and rebuilt on every sync. `core/` and `extension/` are baked into the image. | ||
| - Core skills ARE baked into the Docker image (`COPY skills/core/ ./skills/core/` in Dockerfile.agentbox). They are NOT delivered via the skill bundle — see §2.1. | ||
| - Pod self-destructs after 5 minutes of idle (no SSE connections, no sessions) | ||
| - **CA-fingerprint self-heal**: each pod (and its `-cert` Secret) is stamped with a `<prefix>/ca-fp` label = a fingerprint of the CA that signed its mTLS cert. The runtime reuses a running pod ONLY if that label matches its current CA fingerprint; a mismatch (or a legacy pod with no label) means the CA rotated and the pod can no longer complete mTLS in either direction, so it is deleted and respawned with a fresh cert. This makes "runtime/agentbox cert mismatch after a CA change" self-healing instead of a stuck 403. The CA itself should still be kept stable (persisted Secret); the fingerprint check is the safety net for when it isn't. |
There was a problem hiding this comment.
[Low — multi-replica caveat] The self-heal compares each replica's current CA fingerprint against the pod label. If two runtime replicas ever hold different CAs simultaneously (e.g. mid-rollout right after a CA regen, where replicas loaded different PEM versions), they'd each judge the other's freshly-spawned pods as stale and recycle them → respawn thrash. The "keep the CA stable" guidance avoids this, and the behavior is strictly better than today's stuck 403 regardless. Might be worth one clause here noting the self-heal assumes a single consistent CA across replicas.
…t CA across replicas Addresses #352 review (LikiosSedo): the per-replica fingerprint comparison would thrash if replicas held different CAs simultaneously. The runtime is a singleton today so it can't arise; documented the assumption + that it's still strictly better than the pre-self-heal stuck 403. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @LikiosSedo. Addressed the multi-replica note in Re merge coordination with #350 ( |
LikiosSedo
left a comment
There was a problem hiding this comment.
Approving — re-reviewed after the doc commit (53e95fd).
- Code is correct and self-contained. Verified the heal is complete end to end: on a stale pod the pod is deleted, the
-certSecret is re-minted via the existing409 → delete + recreatepath, and the new pod serves a cert +ca.crtsigned by the current CA — both directions recover in a single respawn, no "recreated but still serving the old cert" gap. Two-layer check (manager.isCertFresh+ the spawner reuse branch) is consistent; back-compat (no-CA spawner → treated fresh, unlabeled legacy pod → recycled once) is handled. - My only review note (multi-replica thrash) is addressed in
53e95fd, with the correct added point that the runtime Deployment is a singleton (replicas: 1,Recreate) so it can't arise today. - CI green (Type Check / Test / Portal Web Test); merges cleanly into
main.
Merge-order suggestion: this conflicts with #350 only in docs/design/invariants.md and src/gateway/agentbox/manager.test.ts (both trivial — see my comment on #350). Since #352 is the smaller, self-contained change, it's the better one to land first; #350 then resolves those two conflicts. src/gateway/agentbox/manager.ts auto-merges correctly either way (verified).
Summary
Make a runtime↔AgentBox mTLS cert mismatch self-healing: when the runtime CA changes, stale AgentBox pods are now automatically recycled with a fresh cert instead of staying Running-but-unreachable (403 / handshake failure) until someone manually deletes them.
Problem
mTLS trust anchors on the
siclaw-runtime-caCA. A cleanhelm upgrade/kubectl set imagedoes not change it (the runtime only reads it;cert-manager.tsloads fromSICLAW_CA_CERT). But it does change when the CA Secret is regenerated —helm template | kubectl apply(the chart'slookupis a no-op in render contexts →genCA), a lookup-less helm op, or a manual delete. When that happens:ca.crttrusting the old CA;Runningbut dead until manually deleted.(Observed on siclaw-inner: orphaned
agentbox-*-certSecrets from before the last CA regeneration.)Solution
Stamp each pod and its
-certSecret with a<prefix>/ca-fplabel = a fingerprint of the signing CA. The runtime reuses a running pod only if that fingerprint matches its current CA; a mismatch (or a legacy pod with no label) is stale → delete + respawn with a cert from the current CA. One respawn fixes both directions at once.Keeping the CA stable (persisted Secret /
generateCa=false+ externalcaSecret) remains the primary recommendation; this is the safety net for when it isn't.Changes
cert-manager.ts:caFingerprint()(sha256 of CA PEM, 16 hex chars)k8s-spawner.ts: stamp pod + Secret label; recycle stale pod in the reuse branch; exposecaFingerprint(); return it fromget()manager.ts:isCertFresh()gate on thegetOrCreateK8sreuse pathtypes.ts/spawner.ts:AgentBoxInfo.caFingerprint+ optionalBoxSpawner.caFingerprint()docs/design/invariants.md§1.3: document the self-heal contractTest Plan
npx tsc --noEmitcleanmanager.test.ts: reuse on fp match; recycle on fp mismatch / missing label / legacy; back-compat when spawner reports no CAk8s-spawner.test.ts: reuse on match (no delete); recycle stale running pod; recycle unlabeled legacy pod; pod+secret stamped with current fp;caFingerprint()reflects cert managersrc/gateway/agentbox/+ server suites green (113 tests)