Skip to content

fix: skip commitsha if not available (requires https)#58

Merged
kdacosta0 merged 1 commit intomainfrom
kdacosta/fix-commitsha-crypto
Dec 1, 2025
Merged

fix: skip commitsha if not available (requires https)#58
kdacosta0 merged 1 commit intomainfrom
kdacosta/fix-commitsha-crypto

Conversation

@kdacosta0
Copy link
Member

@kdacosta0 kdacosta0 commented Nov 28, 2025

Summary by Sourcery

Tests:

  • Update the commit SHA UI test to first check for crypto.subtle availability and skip the test with a warning when running outside HTTPS/localhost.

@sourcery-ai
Copy link

sourcery-ai bot commented Nov 28, 2025

Reviewer's Guide

Adds a runtime check for Web Crypto (crypto.subtle) availability in the commit SHA browser test and conditionally skips the test when running in non-secure contexts where the API is unavailable.

File-Level Changes

Change Details Files
Conditionally skip the CommitSHASearch browser test when the Web Crypto API (crypto.subtle) is unavailable in non-secure contexts.
  • Navigate to the test URL at the start of TestCommitSHASearch to ensure access to window and crypto APIs.
  • Evaluate a small script in the browser context to detect crypto.subtle availability and whether the page is in a secure context.
  • Validate and type-assert the evaluation result from the browser page into a map for further inspection.
  • Log a warning and return early (skipping the test) if crypto.subtle is not available, otherwise proceed with the existing commitSha search test logic.
test/rekorsearchui/rekor_search_sign_verify_test.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • The result of isSecureContext from the Evaluate call is currently unused; if it's not needed, consider removing it from the returned object to simplify the JS snippet, or log/use it explicitly if it’s intended for debugging.
  • TestCommitSHASearch now performs an initial Navigate(bt.URL) before calling performSearch, which may itself navigate; if performSearch already handles navigation, consider reusing that mechanism for the crypto check to avoid redundant page loads.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The result of `isSecureContext` from the `Evaluate` call is currently unused; if it's not needed, consider removing it from the returned object to simplify the JS snippet, or log/use it explicitly if it’s intended for debugging.
- `TestCommitSHASearch` now performs an initial `Navigate(bt.URL)` before calling `performSearch`, which may itself navigate; if `performSearch` already handles navigation, consider reusing that mechanism for the crypto check to avoid redundant page loads.

## Individual Comments

### Comment 1
<location> `test/rekorsearchui/rekor_search_sign_verify_test.go:463-465` </location>
<code_context>
+
+	// Check if crypto.subtle is available (required for commitSha search)
+	// crypto.subtle is only available in secure contexts (HTTPS, localhost)
+	cryptoCheck, err := bt.Browser.Page.Evaluate(`() => {
+		return {
+			available: typeof crypto !== 'undefined' && crypto.subtle !== undefined,
+			isSecureContext: window.isSecureContext
+		};
</code_context>

<issue_to_address>
**suggestion (testing):** Consider asserting on or logging `isSecureContext` to better understand why the test is being skipped.

Since `isSecureContext` is already returned from `Evaluate` but unused, consider either logging it with the skip message (e.g., `isSecureContext=false`) or adding an assertion that in secure environments `isSecureContext` is true whenever `available` is true. This will help distinguish HTTP vs. other failure modes and reduce environment-related flakiness.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Signed-off-by: Kristian Da Costa Menezes <kdacosta@redhat.com>
@kdacosta0 kdacosta0 force-pushed the kdacosta/fix-commitsha-crypto branch from c6a08d6 to 33ddd5a Compare November 28, 2025 14:11
@kdacosta0
Copy link
Member Author

kdacosta0 commented Nov 28, 2025

@kdacosta0 kdacosta0 merged commit d1adaa4 into main Dec 1, 2025
5 checks passed
@kdacosta0 kdacosta0 deleted the kdacosta/fix-commitsha-crypto branch December 1, 2025 14:07
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.

2 participants