feat(deployment): validate confidential compute attestation evidence against hardware vendors#3365
Merged
Merged
Conversation
… cert material The merged attestation evidence shape typed the per-GPU index as `index`, but the provider gateway emits the sidecar payload verbatim whose wire field is `device_index` (chain-sdk AttestationGPUReport / AEP-83 §5). As a result the GPU report label rendered "GPU undefined". Rename the field to match the wire and assert the rendered labels. Also carry `cert_chain` and `auxblob` through AttestationQuote and the downloaded bundle so the CPU report can be verified independently against the hardware vendor (e.g. the AMD VCEK chain), which the upcoming attestation validation (CON-552) requires.
…against hardware vendors Adds POST /v1/confidential-compute/attestation/validate (apps/api): given the downloaded AEP-83 §5 evidence (CPU report + per-GPU reports) and the request nonce, it returns an authenticity verdict per report (valid | invalid | unverifiable) without breaking on a single vendor failure (CON-552). - AMD SEV-SNP: parses the report, fetches the VCEK from AMD KDS (probing candidate products, since the report's cpuid bytes don't reliably name one) or uses an embedded chain, then verifies ARK->ASK->VCEK, the report's ECDSA P-384 signature, and the nonce binding. Revocation (CRL) is not checked — node:crypto has no CRL support — and this is surfaced in the verdict. - NVIDIA: splits each GPU report into its SPDM evidence and embedded device cert chain, posts to NRAS /v3/attest/gpu with the GPU nonce (the first 32 bytes of the tenant nonce), and verifies the returned EAT against NVIDIA's JWKS. - Intel TDX: submits the quote to Intel Trust Authority; without an INTEL_ITA_API_KEY it returns `unverifiable` and makes no outbound call. The NRAS and ITA contracts are not yet confirmed against live services, so both never report a false valid/invalid: anything they cannot confirm degrades to `unverifiable`. The endpoint is authenticated (it makes outbound, partly metered vendor calls).
Adds a "Validate" action to the attestation-evidence modal that posts the fetched evidence to the Console API and renders a per-report verdict badge (Valid / Invalid / Unverifiable) next to the CPU report and each GPU report, with the vendor's explanation underneath (CON-552). Each report renders its own verdict independently, so an unreachable vendor (Unverifiable) for one report never hides another report's verdict, and a whole-request failure surfaces a non-blocking alert while the raw evidence still renders. The modal already only opens for live Confidential Compute leases, so the validation UI inherits that gating.
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughAdds confidential-compute attestation validation across the API and deploy-web. The backend now validates AMD SNP, Intel TDX, and NVIDIA GPU evidence through new schemas, services, and route wiring, while the frontend can submit validation requests and render per-report verdicts. ChangesConfidential Compute Attestation Validation
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: one or more packages not found in the registry. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
apps/deploy-web/src/queries/useAttestationValidationMutation.spec.ts (1)
39-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the
as unknown as HttpClientcast here.This cast bypasses the contract drift check the spec is supposed to give you. Create a typed
mock<HttpClient>()and configureposton the proxy instead.Suggested fix
- const consoleApiHttpClient = mock<HttpClient>({ - post: vi.fn(input.rejection ? () => Promise.reject(input.rejection) : () => Promise.resolve({ data: input.response })) - } as unknown as HttpClient); + const consoleApiHttpClient = mock<HttpClient>(); + consoleApiHttpClient.post.mockImplementation( + input.rejection ? () => Promise.reject(input.rejection) : () => Promise.resolve({ data: input.response }) + );As per coding guidelines, “Use vitest-mock-extended with
mock()andMockProxy<T>for mocking in tests” and “Usemock<T>()instead ofas unknown as <Type>for type casting in tests.”🤖 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 `@apps/deploy-web/src/queries/useAttestationValidationMutation.spec.ts` around lines 39 - 42, The setup helper in useAttestationValidationMutation.spec is bypassing type safety by casting a plain object to HttpClient; replace the `as unknown as HttpClient` usage with a properly typed `mock<HttpClient>()` / `MockProxy<HttpClient>` and configure the `post` method directly on that mock proxy. Keep the existing `setup` helper and `consoleApiHttpClient` symbol, but ensure the test relies on the mocked HttpClient contract instead of a double cast so drift is caught.Source: Coding guidelines
apps/deploy-web/src/components/deployments/AttestationEvidenceModal.spec.tsx (1)
145-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
as unknown asfor the mocked hook results.These casts let the spec keep compiling even when the hook contract changes, which makes the new validation-flow coverage easier to silently break. Return typed mocks instead of casted object literals.
Suggested fix
- const useAttestationQuoteMutation = vi.fn( - () => ({ mutate, data, error: input.error ?? null, isPending: false }) as unknown as ReturnType<typeof DEPENDENCIES.useAttestationQuoteMutation> - ); - const useAttestationValidationMutation = vi.fn( - () => - ({ - mutate: validate, - data: input.validation?.data, - error: input.validation?.error ?? null, - isPending: input.validation?.isPending ?? false - }) as unknown as ReturnType<typeof DEPENDENCIES.useAttestationValidationMutation> - ); + const useAttestationQuoteMutation = vi.fn(() => + mock<ReturnType<typeof DEPENDENCIES.useAttestationQuoteMutation>>({ + mutate, + data, + error: input.error ?? null, + isPending: false + }) + ); + const useAttestationValidationMutation = vi.fn(() => + mock<ReturnType<typeof DEPENDENCIES.useAttestationValidationMutation>>({ + mutate: validate, + data: input.validation?.data, + error: input.validation?.error ?? null, + isPending: input.validation?.isPending ?? false + }) + );As per coding guidelines, “Use vitest-mock-extended with
mock()andMockProxy<T>for mocking in tests” and “Usemock<T>()instead ofas unknown as <Type>for type casting in tests.”🤖 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 `@apps/deploy-web/src/components/deployments/AttestationEvidenceModal.spec.tsx` around lines 145 - 156, The mocked hook results in AttestationEvidenceModal.spec.tsx rely on as unknown as casts, which should be replaced with properly typed mocks. Update the useAttestationQuoteMutation and useAttestationValidationMutation test doubles to use vitest-mock-extended mock<T>() and MockProxy<T> (or equivalent typed mock objects) so the returned values are type-safe without casting away the hook contract, and keep the mock shapes aligned with DEPENDENCIES.useAttestationQuoteMutation and DEPENDENCIES.useAttestationValidationMutation.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 `@apps/api/src/confidential-compute/config/env.config.ts`:
- Around line 23-26: Normalize INTEL_ITA_API_KEY in env.config so
whitespace-only input is treated as missing before it reaches the Intel Trust
Authority client. Update the Zod schema for INTEL_ITA_API_KEY to trim the value
and convert blanks to undefined, so the logic that builds the x-api-key header
does not send invalid whitespace. Use the INTEL_ITA_API_KEY field in the
confidential-compute config as the fix point and keep the existing optional
behavior for truly absent keys.
In `@apps/api/src/confidential-compute/http-schemas/attestation.schema.ts`:
- Line 25: The attestation nonce schema currently only checks for non-empty
input, so malformed values can pass validation and reach vendor verification.
Tighten the `nonce` field in `attestation.schema.ts` so the `z.string()` schema
enforces valid base64 and the expected 64-byte decoded length before any
AMD/Intel/NVIDIA calls are made. Update the `nonce` validator in the attestation
schema to reject invalid encoding or incorrect size at the request boundary.
- Around line 24-33: The attestation request schema currently allows gpu_reports
to be sent with CPU-only tee_platform values, and attestation.service only
processes GPUs when tee_platform ends with "-gpu". Update
VerifyAttestationRequestSchema to enforce the tee_platform/gpu_reports
relationship with a discriminated union or cross-field refinement: CPU-only
platforms like snp/tdx should reject any gpu_reports, and GPU platforms should
require the expected GPU evidence shape. Use the existing
VerifyAttestationRequestSchema and TeePlatformSchema to locate the validation
logic.
In `@apps/api/src/confidential-compute/providers/vendor-clients.provider.ts`:
- Around line 15-27: The vendor HTTP client registrations for
AMD_KDS_HTTP_CLIENT, NVIDIA_NRAS_HTTP_CLIENT, and INTEL_ITA_HTTP_CLIENT are
missing explicit request timeouts, so the axios clients can hang on stalled
attestation calls. Update the createHttpClient calls inside
vendor-clients.provider.ts to pass a timeout for each vendor, using the same
instancePerContainerCachingFactory setup and tuning the values per vendor if
needed. Keep the change localized to the three provider registrations so the
timeout is applied whenever these clients are resolved from
CONFIDENTIAL_COMPUTE_CONFIG.
In `@apps/api/src/confidential-compute/services/amd-snp/amd-snp.service.ts`:
- Around line 72-76: The trust check in AMD SNP certificate resolution currently
accepts caller-supplied PEM chains as sufficient, which can let forged
self-signed chains verify as valid. Update `#resolveChain` and the related
verification flow in amd-snp.service to require a pinned AMD trust anchor:
either validate the embedded ARK against a known AMD root or resolve/pin ASK+ARK
from KDS before accepting the embedded VCEK. Ensure the report signature is only
verified after the chain is anchored to AMD, and adjust the amd-snp.service.spec
cases that currently pass with a locally generated chain to reflect the new
requirement.
In `@apps/api/src/confidential-compute/services/attestation.service.ts`:
- Around line 63-66: The attestation failure path in attestation.service.ts is
leaking upstream exception text into the client-facing verdict via the detail
derived from error.message. Update the catch block in attestation.service.ts so
verify/attestation errors still log the raw exception through this.logger.error
with ATTESTATION_VERIFY_ERROR, but return toUnverifiable with a generic
client-safe message instead of propagating error.message. Keep the response
wording independent of the thrown Error content and preserve the existing
fallback for non-Error cases.
In `@apps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.ts`:
- Around line 69-72: The Intel Trust Authority verdict handling in
intel-tdx.service.ts is hard-coding checks.nonceMatch to true, so the token is
not actually bound to the request nonce. Update the verification flow in the
service method that builds the verdict to read the JWT nonce/runtime_data claim
from ITA’s response and compare it against input.nonce before returning "valid";
if they do not match, mark the verdict invalid and set checks.nonceMatch
accordingly.
In `@apps/api/src/confidential-compute/services/jwt-verify.ts`:
- Around line 20-22: The JWT verification logic in jwt-verify.ts currently falls
back to jwks.keys[0] when decoded.header.kid is missing or does not match, which
makes key selection depend on JWKS ordering. Update the matching logic in the
jwt verification flow so it only uses a fallback when the vendor JWKS contains
exactly one key; otherwise require an explicit kid match and fail on ambiguity.
Keep the change localized around the jwks.keys lookup and the existing "No
matching key in the vendor JWKS" error path.
In `@apps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.ts`:
- Around line 44-45: The nonce validation in nvidia-gpu.service.ts is incomplete
because gpuNonceHex is used for the NRAS request but nonceMatch is always set to
true. Update the attestation flow in the relevant method to compare the EAT
nonce/challenge claim against gpuNonceHex before setting the report as valid,
and only mark the result valid when that comparison succeeds. Use the existing
gpuNonceHex and nonceMatch logic in the service to locate and replace the
hard-coded success path.
---
Nitpick comments:
In
`@apps/deploy-web/src/components/deployments/AttestationEvidenceModal.spec.tsx`:
- Around line 145-156: The mocked hook results in
AttestationEvidenceModal.spec.tsx rely on as unknown as casts, which should be
replaced with properly typed mocks. Update the useAttestationQuoteMutation and
useAttestationValidationMutation test doubles to use vitest-mock-extended
mock<T>() and MockProxy<T> (or equivalent typed mock objects) so the returned
values are type-safe without casting away the hook contract, and keep the mock
shapes aligned with DEPENDENCIES.useAttestationQuoteMutation and
DEPENDENCIES.useAttestationValidationMutation.
In `@apps/deploy-web/src/queries/useAttestationValidationMutation.spec.ts`:
- Around line 39-42: The setup helper in useAttestationValidationMutation.spec
is bypassing type safety by casting a plain object to HttpClient; replace the
`as unknown as HttpClient` usage with a properly typed `mock<HttpClient>()` /
`MockProxy<HttpClient>` and configure the `post` method directly on that mock
proxy. Keep the existing `setup` helper and `consoleApiHttpClient` symbol, but
ensure the test relies on the mocked HttpClient contract instead of a double
cast so drift is caught.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c85ba441-58f1-433b-9204-5120cbba4ae6
📒 Files selected for processing (27)
apps/api/src/confidential-compute/config/env.config.tsapps/api/src/confidential-compute/controllers/attestation.controller.tsapps/api/src/confidential-compute/http-schemas/attestation.schema.tsapps/api/src/confidential-compute/index.tsapps/api/src/confidential-compute/providers/config.provider.tsapps/api/src/confidential-compute/providers/vendor-clients.provider.tsapps/api/src/confidential-compute/routes/attestation.router.tsapps/api/src/confidential-compute/routes/index.tsapps/api/src/confidential-compute/services/amd-snp/amd-kds.client.tsapps/api/src/confidential-compute/services/amd-snp/amd-snp.service.spec.tsapps/api/src/confidential-compute/services/amd-snp/amd-snp.service.tsapps/api/src/confidential-compute/services/amd-snp/snp-report.parser.spec.tsapps/api/src/confidential-compute/services/amd-snp/snp-report.parser.tsapps/api/src/confidential-compute/services/attestation.service.spec.tsapps/api/src/confidential-compute/services/attestation.service.tsapps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.spec.tsapps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.tsapps/api/src/confidential-compute/services/jwt-verify.tsapps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.spec.tsapps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.tsapps/api/src/routers/open-api-handlers.tsapps/deploy-web/src/components/deployments/AttestationEvidenceModal.spec.tsxapps/deploy-web/src/components/deployments/AttestationEvidenceModal.tsxapps/deploy-web/src/queries/useAttestationValidationMutation.spec.tsapps/deploy-web/src/queries/useAttestationValidationMutation.tsapps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.tsapps/deploy-web/src/utils/confidentialCompute.ts
stalniy
reviewed
Jun 26, 2026
stalniy
approved these changes
Jun 26, 2026
stalniy
left a comment
stalniy
left a comment
Contributor
There was a problem hiding this comment.
make sure that this validation will not block event loop and will not cause OOM. Luna said one verification payload is 12kb, should be a big issue
…iers - collapse the dead claim loop in extractTdxResult to a direct attestation_result check - drop the redundant signatureValid override in the NVIDIA invalid verdict - hoist the PEM cert-split regex to a module-level constant
baktun14
added a commit
to akash-network/console-air
that referenced
this pull request
Jun 26, 2026
…(CON-552) (#39) * feat(deployment): validate confidential compute attestation evidence (CON-552) Port akash-network/console#3365 and #3364 into the self-custody fork. From a running Confidential Compute lease's attestation-evidence modal the tenant can now Validate the downloaded evidence against the hardware vendors and see a per-report verdict (valid | invalid | unverifiable). Backend has no apps/api in this fork, so the verifier is self-hosted in a Next.js API route (src/lib/nextjs/confidential-compute) rather than the upstream NestJS module: - AMD SEV-SNP (fully functional): parses the SNP report, resolves the VCEK chain (embedded, else fetched from AMD KDS by probing candidate products), then verifies ARK->ASK->VCEK, the report's ECDSA P-384 signature and the nonce binding via node:crypto. CRL revocation is not checked. - NVIDIA: splits each GPU report into SPDM evidence + device cert chain, posts to NRAS, verifies the returned EAT against NVIDIA's JWKS. - Intel TDX: submits the quote to Intel Trust Authority; without INTEL_ITA_API_KEY returns unverifiable and makes no outbound call. Each report verifies independently (per-report error isolation), and the NRAS/ITA contracts are unconfirmed so both degrade to unverifiable rather than ever reporting a false valid/invalid. POST /api/confidential-compute/attestation/validate is the local route; the frontend mutation posts the evidence same-origin via publicConsoleApiHttpClient, so it never leaves the deployment's server. Also (CON-540 follow-up, #3364): rename GpuAttestationReport.index -> device_index to match the provider wire field (was rendering "GPU undefined"), and carry cert_chain/auxblob through AttestationQuote and the downloaded bundle for independent CPU-report verification. * fix(deployment): parse real NVIDIA NRAS response shape and never false-fail on rejection Verified against the live NRAS /v3/attest/gpu service with real SNP-GPU evidence. NRAS returns [["JWT", overallEat], { "GPU-N": deviceEat }], not a bare token — so extractEat read null and every GPU report degraded to "no attestation token". Parse the tagged tuple and the per-device map. NRAS reports a rejected/malformed submission (e.g. INVALID_CERTIFICATE_CHAIN) as a structured x-nvidia-error-details claim in the JWKS-signed per-device token, with the overall result set to false. Treat that as `unverifiable` with the NVIDIA reason surfaced, never a false `invalid` — NRAS could not complete the check, which is not proof the GPU is counterfeit. Also enforce the verified EAT's eat_nonce binding when present. Confirmed correct (no change needed): evidence must be base64 (hex -> INVALID_EVIDENCE_FORMAT) and the certificate must be base64-of-PEM. Note: today's GB202 sample still returns INVALID_CERTIFICATE_CHAIN from NRAS (the embedded chain has an issuer/subject DN mismatch), so the GPU verdict is a precise `unverifiable` rather than a real valid; confirming the Blackwell cert/ evidence contract against NVIDIA's nvtrust SDK is a follow-up.
…and request nonce Closes three trust gaps that could let forged or replayed attestation evidence read as authentic (CodeRabbit findings + console-air #39 port): - AMD SEV-SNP: the verifier trusted a caller-supplied embedded ARK/ASK chain, so a self-consistent forged chain (attacker's own self-signed ARK) verified as `valid`. Trust is now always anchored to AMD KDS: the genuine ARK/ASK are resolved from KDS and the VCEK (embedded or fetched) is accepted only once it is signed by that AMD-issued ASK. Adds a forged-chain regression test. - NVIDIA: compare the EAT `eat_nonce` claim against the request nonce and report `invalid` on mismatch; surface a JWKS-verified per-device NRAS error (e.g. INVALID_CERTIFICATE_CHAIN) as `unverifiable` rather than a false `invalid`; parse the live `["JWT", token]`-tagged NRAS response shape. - Intel TDX: enforce the token's runtime_data/nonce claim against the request nonce when present, instead of hard-coding nonceMatch to true. - jwt-verify: only fall back to the sole JWKS key; require an explicit kid match when the vendor publishes more than one key, so verification no longer depends on JWKS ordering.
…endor calls Addresses CodeRabbit findings on the attestation validate endpoint: - Validate the nonce at the request boundary (canonical base64 decoding to exactly 64 bytes) so a malformed challenge is rejected with a 400 instead of triggering metered vendor calls that can only ever return invalid. - Reject gpu_reports on CPU-only platforms (snp, tdx) instead of silently dropping them, which previously produced a misleading overall verdict. - Trim INTEL_ITA_API_KEY so a whitespace-only value is treated as unconfigured rather than sent as an invalid x-api-key header. - Set an explicit 10s timeout on the AMD/NVIDIA/Intel HTTP clients so a stalled vendor degrades the report to unverifiable quickly instead of hanging. - Keep upstream exception text out of the client-facing verdict (it is still logged) to avoid leaking vendor URLs and library internals.
…API client Per review feedback, drop the hand-written useAttestationValidationMutation hook and call the validate endpoint through the generated console API react-query client (api.v1.validateConfidentialComputeAttestation.useMutation). Regenerates the console-api-types SDK to expose the operation and its request/response types, and sources the per-report verdict types in the modal from the generated schema. No behavior change: the modal still posts the downloaded evidence and renders a verdict badge per report.
…ate-attestation-evidence # Conflicts: # apps/deploy-web/src/components/deployments/AttestationEvidenceModal.tsx # apps/deploy-web/src/utils/confidentialCompute.ts
Contributor
Author
|
Re: event loop / OOM on the validate path — it's bounded. The request body is capped at 256kb (bodyLimit middleware) and the per-report payloads are small (~12kb). The only CPU-bound work is Also pushed fixes for the CodeRabbit findings (AMD KDS trust anchor, NVIDIA/Intel nonce binding, JWKS ambiguity, request/nonce validation, vendor timeouts, error-leak) and migrated the validation call to the generated console client per your suggestion (dropped the custom hook). Threads resolved. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@apps/api/src/confidential-compute/services/amd-snp/amd-snp.service.ts`:
- Around line 87-90: The VCEK selection in amd-snp.service.ts currently only
calls `#fetchVcek`() when embeddedCerts is empty, so a non-anchoring embedded cert
set incorrectly blocks KDS fallback. Update the logic around the
embeddedCerts/safeVerify check to detect when no embedded certificate verifies
against ask.publicKey and then fall back to `#fetchVcek`(product, parsed) before
continuing; keep the existing continue only when both embedded and KDS lookup
fail.
In `@apps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.ts`:
- Around line 64-71: The nonce-binding check in IntelTdxService should not treat
a missing runtime-data/nonce claim as a successful match. Update
nonceBindingHolds() and the verdict path in IntelTdxService.#verdict so that an
absent comparable claim yields "unverifiable" rather than "valid" with
nonceMatch: true, while an explicit mismatch still returns "invalid"; make the
same adjustment anywhere else the same nonce-binding logic is used, including
the other referenced block.
In `@apps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.ts`:
- Around line 83-91: The nonce binding check in nvidia-gpu.service.ts’s
attestation flow is too permissive because the verdict path in the verification
logic can still reach valid when payload.eat_nonce is missing. Update the
handling around the existing eat_nonce comparison so that the main verification
branch in the NVIDIA attestation method treats a missing nonce as unverifiable
rather than setting nonceMatch to true; only return valid when eat_nonce is
present and matches gpuNonceHex, otherwise return an unverifiable verdict with
clear nonce-related details.
- Around line 148-152: The JWT parsing logic in nvidia-gpu.service’s token
normalization path is returning the tag string instead of the actual token for a
top-level tagged tuple. Update the array handling in the JWT extraction helper
so it checks for the direct ["JWT", token] shape before the generic array[0]
string return, and return data[1] when the tuple matches; keep the existing
nested tagged-array handling intact.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b6eaae88-c33a-4b94-b6e5-065e6d2ddadb
📒 Files selected for processing (18)
apps/api/src/confidential-compute/config/env.config.tsapps/api/src/confidential-compute/http-schemas/attestation.schema.tsapps/api/src/confidential-compute/providers/vendor-clients.provider.tsapps/api/src/confidential-compute/services/amd-snp/amd-snp.service.spec.tsapps/api/src/confidential-compute/services/amd-snp/amd-snp.service.tsapps/api/src/confidential-compute/services/attestation.service.spec.tsapps/api/src/confidential-compute/services/attestation.service.tsapps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.spec.tsapps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.tsapps/api/src/confidential-compute/services/jwt-verify.tsapps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.spec.tsapps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.tsapps/deploy-web/src/components/deployments/AttestationEvidenceModal.spec.tsxapps/deploy-web/src/components/deployments/AttestationEvidenceModal.tsxapps/deploy-web/src/components/sdl/RegionSelect/RegionSelect.tsxapps/deploy-web/src/utils/urlUtils.spec.tspackages/console-api-types/src/operations.gen.tspackages/console-api-types/src/schema.d.ts
✅ Files skipped from review due to trivial changes (3)
- apps/deploy-web/src/utils/urlUtils.spec.ts
- apps/deploy-web/src/components/sdl/RegionSelect/RegionSelect.tsx
- packages/console-api-types/src/operations.gen.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/api/src/confidential-compute/providers/vendor-clients.provider.ts
- apps/api/src/confidential-compute/services/jwt-verify.ts
- apps/api/src/confidential-compute/config/env.config.ts
- apps/api/src/confidential-compute/services/amd-snp/amd-snp.service.spec.ts
- apps/api/src/confidential-compute/http-schemas/attestation.schema.ts
- apps/api/src/confidential-compute/services/attestation.service.ts
- apps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.spec.ts
Addresses the CodeRabbit re-review on the verifiers: - AMD SEV-SNP: a present-but-wrong embedded chain no longer blocks the KDS fallback. Per product we accept an embedded VCEK only if it is signed by that product's AMD ASK, otherwise we fetch the per-chip VCEK from KDS. - NVIDIA / Intel: when the verified token carries no nonce-binding claim (eat_nonce / runtime_data) we can no longer confirm the evidence is bound to this request, so the verdict degrades to `unverifiable` instead of asserting `valid` with nonceMatch: true. An explicit mismatch still returns `invalid`. - NVIDIA extractEat: handle a bare `["JWT", token]` tuple so the token is returned instead of the "JWT" media-type tag.
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@apps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.spec.ts`:
- Around line 36-42: The Intel TDX spec test is only asserting an invalid
verdict, which can also happen because of nonce binding, so make the assertion
target the TCB-status failure path in IntelTdxService.verify. Reuse a single
request nonce for both signToken and service.verify, then add an explicit
assertion that nonceMatch remains true while verdict.status is invalid so the
test proves the invalid result comes from attester_tcb_status rather than nonce
mismatch.
In `@apps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.ts`:
- Around line 117-118: The nonce comparison in intel-tdx.service.ts is making
the base64 path case-insensitive by lowercasing the input before comparing
against nonceB64, which can incorrectly match mutated runtime data. Update the
comparison logic in the nonce validation method so nonceB64 is compared exactly
as-is, while keeping only the nonceHex comparison case-insensitive; use the
existing normalized/value handling in the service to locate and adjust the
check.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0f0c75b7-27e6-40e0-866f-1e9aa80a7050
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
apps/api/src/confidential-compute/services/amd-snp/amd-snp.service.spec.tsapps/api/src/confidential-compute/services/amd-snp/amd-snp.service.tsapps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.spec.tsapps/api/src/confidential-compute/services/intel-tdx/intel-tdx.service.tsapps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.spec.tsapps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/api/src/confidential-compute/services/amd-snp/amd-snp.service.spec.ts
- apps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.spec.ts
- apps/api/src/confidential-compute/services/nvidia-gpu/nvidia-gpu.service.ts
- apps/api/src/confidential-compute/services/amd-snp/amd-snp.service.ts
Addresses the CodeRabbit re-review: - nonceBinding compared the base64 nonce case-insensitively, but base64 is case-sensitive, so a case-mutated runtime_data could be treated as bound. Compare the base64 form exactly; keep only the hex form case-insensitive. - Pin the bad-TCB-status test to a single request nonce and assert nonceMatch stayed true, so the invalid verdict can't pass for a nonce-mismatch reason.
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
2 participants
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.
Why
Closes CON-552. CON-540 (#3359) lets a tenant download a Confidential Compute lease's hardware-signed attestation evidence, but raw blobs are not actionable on their own. This turns that evidence into a clear pass/fail per report in the deployment detail view, validated against the hardware vendors (AMD SEV-SNP / Intel TDX for the CPU report, NVIDIA for each GPU report). Authenticity-only: it proves the evidence came from genuine, nonce-bound vendor hardware; it does not yet check workload measurement.
What
Backend (
apps/api/src/confidential-compute/): new authenticated endpointPOST /v1/confidential-compute/attestation/validatethat takes the downloaded evidence + the request nonce and returns a verdict per report (valid|invalid|unverifiable), with per-report error isolation so one vendor failing never fails the whole response.node:crypto. Revocation (CRL) is not checked (node has no CRL support) and this is surfaced in the verdict detail./v3/attest/gpuwith the GPU nonce (the first 32 bytes of the tenant nonce, which the report embeds), and verifies the returned EAT against NVIDIA's JWKS.INTEL_ITA_API_KEYit returnsunverifiableand makes no outbound call.Frontend (
apps/deploy-web): a "Validate" action in the existing attestation-evidence modal posts the evidence to the Console API and renders a verdict badge (Valid / Invalid / Unverifiable) next to the CPU report and each GPU report, with the vendor's reason underneath. Each report renders its verdict independently, and a whole-request failure shows a non-blocking alert while the raw evidence still renders.Also fixed (CON-540 follow-up): the merged evidence shape typed the per-GPU index as
index, but the provider gateway emitsdevice_index(chain-sdkAttestationGPUReport), so the GPU label rendered "GPU undefined". Renamed to match the wire and now carrycert_chain/auxblobthrough the quote and download bundle (needed for independent verification).Notes for the reviewer
ui_confidential_computeflag (the download already only renders for live CC leases with an on-chain TEE type). The validation UI lives in that same modal and inherits the gating, so no flag was re-added. Flag this if the original AC's flag requirement still stands.valid/invalid— anything they cannot confirm degrades tounverifiable. In the current environment (no ITA key; NRAS/ITA contracts unconfirmed) only AMD produces real verdicts today.INTEL_ITA_API_KEY, and confirm the NRAS request/EAT-claim contract against the live service (and that the sidecar embeds the per-GPU cert, which the latest sample does).Testing
apps/api: 37 unit tests for the new module, including real-crypto AMD verification (an openssl-generated P-384 chain + a signed report), the orchestrator's per-report isolation, and JWKS-verified NVIDIA/Intel paths.apps/deploy-web: validation mutation + modal tests (per-report verdicts, isolation, non-blocking error), plus the correcteddevice_indexrendering.lint --quietandtsc --noEmitclean for the changed files.Summary by CodeRabbit