Skip to content

fix(verifier): handle missing ACME hist_keys and guest_agent_info gracefully#38

Merged
kingsleydon merged 1 commit intomainfrom
fix/verifier-error-handling
Mar 24, 2026
Merged

fix(verifier): handle missing ACME hist_keys and guest_agent_info gracefully#38
kingsleydon merged 1 commit intomainfrom
fix/verifier-error-handling

Conversation

@kingsleydon
Copy link
Copy Markdown
Collaborator

Summary

  • Add AcmeInfoSchema Zod validation for Gateway ACME info API responses, replacing unsafe as AcmeInfo type assertion that caused undefined is not an object (evaluating 'historicalKeys[0]') crashes
  • Add null/empty check for hist_keys before accessing historicalKeys[0] in verifyCertificateKey()
  • Fall back to legacy stub verifiers when kms_guest_agent_info or gateway_guest_agent_info is missing in SystemInfo, even for KMS versions >= 0.5.3, preventing kms_guest_agent_info not available errors

Test plan

  • Verify apps with missing hist_keys in ACME info return a descriptive error instead of crashing
  • Verify apps with missing kms_guest_agent_info gracefully degrade to legacy stubs
  • Verify normal verification flow still works end-to-end

🤖 Generated with Claude Code

…cefully

- Add AcmeInfoSchema for runtime validation of Gateway ACME info responses
  instead of unsafe `as AcmeInfo` type assertion
- Add null check for hist_keys before accessing historicalKeys[0] in
  verifyCertificateKey to prevent runtime crash
- Fall back to legacy stub verifiers when kms_guest_agent_info or
  gateway_guest_agent_info is missing, even if KMS version >= 0.5.3

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
trust-center Ready Ready Preview, Comment Mar 24, 2026 8:27am

Request Review

@kingsleydon kingsleydon merged commit 58212ee into main Mar 24, 2026
4 checks passed
Comment on lines +43 to +46
const useLegacyStubs =
!supportsOnchainKms(systemInfo.kms_info.version) ||
!systemInfo.kms_guest_agent_info ||
!systemInfo.gateway_guest_agent_info
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The code falls back to a stub verifier if gateway_guest_agent_info is missing, unnecessarily skipping verification, as GatewayVerifier can handle this case internally.
Severity: MEDIUM

Suggested Fix

Modify the condition to only fall back to the stub verifier for the PhalaCloudKmsVerifier when kms_guest_agent_info is missing. Allow the GatewayVerifier to proceed even without gateway_guest_agent_info, as it has its own internal fallback logic.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/verifier/src/verifierChain.ts#L43-L46

Potential issue: The code at `verifierChain.ts` introduces a fallback to a stub verifier
when `gateway_guest_agent_info` is missing from the system information. This is an
unnecessary security degradation because the `GatewayVerifier` is capable of handling
this absence by fetching the required information from a Gateway RPC endpoint. By
falling back to the stub verifier, which performs no actual validation and simply
returns a hardcoded success result, the system bypasses a legitimate verification path
that could have otherwise completed successfully.

Did we get this right? 👍 / 👎 to inform future reviews.

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.

1 participant