chore: v04 hardening#128
Merged
Merged
Conversation
… tests NormalizeEndpoint had a TODO marker about full url.Parse coverage, plus two real edge-case bugs: tcp:// without an explicit port silently dropped the default 50051, and IPv6 literals (tcp://[::1]) were never exercised. Reshape so all three schemes (tcp/http/https) flow through url.Parse into a single host string, then a single defaulting step appends :50051 when no port is already present. IPv6 needs a special-case skip of net.JoinHostPort because url.Parse already brackets `[::1]` and JoinHostPort would double-bracket a host whose name itself contains `:`. Also reject empty / scheme-only inputs with a typed VAULT_BAD_ENDPOINT error instead of returning a useless empty string downstream. Adds endpoint_test.go covering 12 happy-path cases (tcp/http/https with and without ports, schemeless host:port, schemeless host, IPv4 loopback, IPv6 literal with and without port, whitespace trimming) and 3 error cases (empty, whitespace-only, scheme-only). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The TODO block above LifecycleService.ReloadPipelines documented the function as "currently a no-op for state recovery" and instructed the reader that /rune:activate cannot recover from dormant — both incorrect. Task #28 (PR #117) wired Manager.SetReloadFunc / Manager.Retrigger so ReloadPipelines re-spawns RunBootLoop from a terminal Dormant state, and the smoke test in this branch history confirmed end-to-end recovery without a process restart. The stale comment is load-bearing — a recent parity audit took the TODO at face value and reported reload_pipelines as incomplete in Go, flagging it as a blocker for deleting the Python tree. Replacing it with the actual contract avoids the same trap on future audits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The recall pipeline issues 8 external-IO calls (3 to embedder, 2 to envector, 3 to vault) and was relying on gRPC keepalive alone to fail hung dependencies. Keepalive only catches connection-level death; a server that ACKs the request, holds the stream, and never replies will stall the whole MCP request indefinitely — a real risk with FHE operations on stressed envector clusters or AES decrypts batching too many entries on a slow Vault host. Wraps each external call in a context.WithTimeout derived from the caller's context. Five constants (one per hop type) with explicit rationale in the doc block: embedderCallTimeout 10s runed cold-start tolerance envectorScoreTimeout 30s FHE inner-product (heaviest hop) envectorMetadataTimeout 15s shard/row cipher lookup vaultDecryptScoresTimeout 30s FHE decrypt (matches Score) vaultDecryptMetadataTimeout 30s AES-only, batch tolerant The cancel() is called immediately after each RPC returns so the timer resource releases as soon as the call resolves; this matches the pattern Go's context docs recommend for non-deferred per-call contexts. A hung dependency now surfaces as a typed DeadlineExceeded error in seconds instead of stalling the request. Drops the TODO at L41 that flagged this gap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ong/chore/v04-hardening # Conflicts: # internal/service/lifecycle.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
a. Vault endpoint 정규화 (internal/adapters/vault/endpoint.go)
문제: NormalizeEndpoint 가 미완성. 두 edge case 가 silently broken:
변경:
영향: Vault endpoint 입력 robustness. 사용자가 tcp://vault.cryptolab.co.kr 처럼 포트 안 적어도 동작.
b. Stale TODO 주석 정리 (internal/service/lifecycle.go:489)
변경: TODO 블록을 실제 contract 설명으로 교체.
c. Recall external-IO timeouts (internal/service/recall.go)
문제: recall 파이프라인이 8개 외부 RPC 를 호출하는데 (Embedder 3, Envector 2, Vault 3) gRPC keepalive 만 의존. keepalive 는 connection 끊김만 감지 — 서버가 ACK 보내고 stream 만 잡고 응답 안 주면 무한 hang. FHE 부하 큰 envector cluster 나 느린 Vault 호스트에서 실재 risk.
변경: