feat: validate attestation gateway even on disabled#213
Merged
Conversation
paolodamico
requested review from
aurel-fr,
murph and
thomas-waite
as code owners
July 16, 2026 00:32
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43cdd77a7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0c22d3f. Configure here.
Alex-Vygodchikov
previously approved these changes
Jul 16, 2026
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
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.

Changes
DISABLE_ATTESTATION_GATEWAYif set, instead of disabling attestation checks altogether, the service will verify the attestation gateway checks in full, log a warning and let the request continue. This way any failures can be diagnosed before turning on enforcement.attestation-failureheader will be returned to the client to let it know that their request would've been rejected and be able to debug integrity computation errors. cc @SeanROlszewski @Guardiola31337DISABLE_ATTESTATION_GATEWAYin favor ofDISABLE_ATTESTATION_GATEWAY_ENFORCEMENT(naming; old version is still supported).Rationale
The env var that allowed disabling the attestation gateway checks altogether, which is problematic. The raison d'être of this config is to be able to monitor issues and allow bypassing for a progressive roll-out. If all checks are skipped, then there's no way to monitor the correct functionality of this functionality.
Note
High Risk
Changes how attestation bypass env vars behave (validation always runs) and can allow unauthenticated traffic when enforcement is disabled, which is security-critical for progressive rollout.
Overview
Attestation middleware always runs validation instead of skipping checks when a disable flag is set. A new
do_not_enforcemode (fromDISABLE_ATTESTATION_GATEWAY_ENFORCEMENT, with legacyDISABLE_ATTESTATION_GATEWAYstill honored) logs failures, still returns HTTP 200 to the handler, and adds anattestation-failureresponse header with the message the client would have seen under enforcement.Constructor/config cleanup:
AttestationGateway::newdropsAttestationGatewayConfig/enabledin favor of(base_url, env, do_not_enforce). Missing or badattestation-tokenheaders are collapsed toinvalid_attestation_token_header(header helper now returnsOption).ErrorResponsegainscode()/message()for that header.Unit and integration tests cover non-enforcing vs enforcing paths and updated error codes.
Reviewed by Cursor Bugbot for commit 6d6ea8c. Bugbot is set up for automated code reviews on this repo. Configure here.