Skip to content

ci(maestro): bump WalletConnect/actions for redesigned /collect IC form#391

Open
ignaciosantise wants to merge 6 commits into
developfrom
ci/bump-actions-collect-ic-redesign
Open

ci(maestro): bump WalletConnect/actions for redesigned /collect IC form#391
ignaciosantise wants to merge 6 commits into
developfrom
ci/bump-actions-collect-ic-redesign

Conversation

@ignaciosantise

@ignaciosantise ignaciosantise commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Bump WalletConnect/actions maestro refs for the redesigned /collect IC form

WalletConnect/buyer-experience PR #914 redesigns the hosted KYC webview (the /collect route the sample wallet opens in a WebView). The native Maestro pay flows drive that webview by visible text, so they were updated in WalletConnect/actions#106 (submit is now "Confirm", the "Confirm your details" dialog is gone, and consent is an inline "I agree" checkbox). The verified KYC sequence is: scroll to "I agree" → tap it → scroll to "Confirm" → tap it.

This PR pins every WalletConnect/actions/maestro/* ref (pay-tests / setup / permit2-reset) to the merged-master SHA that contains those flow changes.

✅ Ready to merge — remaining requirement

Validation

Android Maestro pay runs were dispatched on this branch against the merged action and exercised the new KYC flow (scroll + tap "I agree", scroll + tap "Confirm").

🤖 Generated with Claude Code

…llect IC form

Provisionally pinned to the branch tip of WalletConnect/actions#106 (KYC
/collect inline IC form redesign). Re-pin to the merged-master SHA before
merging, and only after buyer-experience #914 is deployed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown

Claude finished @ignaciosantise's task in 1m 8s —— View job


Review Complete

  • Gather context
  • Read changed files
  • Run breaking changes subagent
  • Run data classification subagent
  • Post review findings

✅ No issues found

Summary: All 6 changed lines are SHA pin bumps for WalletConnect/actions composite actions (maestro/pay-tests, maestro/setup, maestro/permit2-reset) across the Android and iOS jobs. SHAs are pinned by full commit hash (good practice). No pull_request_target, no data classification issues, no breaking changes.

One observation worth noting: prior to this PR, maestro/setup was pinned to a different SHA (0730e32) than pay-tests/permit2-reset (3fd66ca). This PR aligns all three to the same provisional SHA — that alignment is correct and the previous divergence was likely an earlier unintentional drift.

The provisional SHA concern (a6436df is a branch tip, not a merged-master commit) is already well-documented in both the PR description and inline comments. Merge sequencing requirements are clearly spelled out.

ignaciosantise and others added 5 commits July 3, 2026 13:03
Still provisional — re-pin to the merged-master SHA before merge. Advances
a6436df -> bb4ed70 so CI exercises the text-based consent ("0") and "Confirm"
selectors; the earlier testIDs were not exposed in the /collect WebView.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…llect-ic-redesign

# Conflicts:
#	.github/workflows/ci_e2e_pay_tests.yml
…merged)

#106 now has master (#107 skip-metrics) merged in, so this single SHA carries
both the redesigned /collect flow fixes and the skip-metrics marker support.
Still provisional — re-pin to the merged-master SHA once #106 lands.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#106 now taps the "I agree" consent label instead of the "0" value node,
which did not toggle consent in CI. Still provisional — re-pin to the
merged-master SHA once #106 lands.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WalletConnect/actions#106 merged to master (ddc7c0f) — the redesigned /collect
inline IC flow fixes. Pin to the merged-master SHA and drop the provisional note.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ignaciosantise ignaciosantise marked this pull request as ready for review July 3, 2026 20:52
Copilot AI review requested due to automatic review settings July 3, 2026 20:52
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Claude finished @ignaciosantise's task in 2m 31s —— View job


🔄 Incremental Review:

  • Gather context
  • Read changed file
  • Run breaking changes subagent
  • Run data classification subagent
  • Post review findings

✅ No new issues in the SHA-pinned lines themselves. One pre-existing issue surfaced in the iOS job (not introduced by this PR).

Found 1 issue(s)

Issue 1: iOS Maestro step passes WPAY secrets as inline CLI arguments

ID: dcl-ci_e2e_pay_tests-secrets-cli-args-a4f2
File: .github/workflows/ci_e2e_pay_tests.yml:401
Severity: MEDIUM
Category: security / data_classification

Context:

  • Pattern: The iOS Run Maestro iOS tests step (lines 394–409) expands six ${{ secrets.WPAY_* }} expressions directly into the maestro CLI argument string. The Android job (lines 127–153) does this correctly — secrets are in the step env: block and passed as shell variables.
  • Risk: GitHub Actions expands ${{ secrets.* }} before the shell starts, placing literal secret values in the process command-line string visible via /proc/<pid>/cmdline. Any process in the same job — including the composite WalletConnect/actions/maestro/* steps — can enumerate running processes and read those values.
  • Impact: WPAY customer/merchant API keys exposed to a compromised or supply-chain-attacked composite action running in the same job.
  • Trigger: Any step in the iOS job that calls ps or reads /proc after the maestro process starts.

Recommendation: Mirror the Android pattern — declare secrets in env:, reference as shell vars:

- name: Run Maestro iOS tests
  id: maestro_ios
  env:
    WPAY_CUSTOMER_KEY_SINGLE_NOKYC: ${{ secrets.WPAY_CUSTOMER_KEY_SINGLE_NOKYC }}
    WPAY_MERCHANT_ID_SINGLE_NOKYC: ${{ secrets.WPAY_MERCHANT_ID_SINGLE_NOKYC }}
    WPAY_CUSTOMER_KEY_MULTI_NOKYC: ${{ secrets.WPAY_CUSTOMER_KEY_MULTI_NOKYC }}
    WPAY_MERCHANT_ID_MULTI_NOKYC: ${{ secrets.WPAY_MERCHANT_ID_MULTI_NOKYC }}
    WPAY_CUSTOMER_KEY_MULTI_KYC: ${{ secrets.WPAY_CUSTOMER_KEY_MULTI_KYC }}
    WPAY_MERCHANT_ID_MULTI_KYC: ${{ secrets.WPAY_MERCHANT_ID_MULTI_KYC }}
  run: |
    maestro test \
      --env APP_ID=com.walletconnect.flutterwallet.internal \
      --env WPAY_CUSTOMER_KEY_SINGLE_NOKYC=$WPAY_CUSTOMER_KEY_SINGLE_NOKYC \
      --env WPAY_MERCHANT_ID_SINGLE_NOKYC=$WPAY_MERCHANT_ID_SINGLE_NOKYC \
      --env WPAY_CUSTOMER_KEY_MULTI_NOKYC=$WPAY_CUSTOMER_KEY_MULTI_NOKYC \
      --env WPAY_MERCHANT_ID_MULTI_NOKYC=$WPAY_MERCHANT_ID_MULTI_NOKYC \
      --env WPAY_CUSTOMER_KEY_MULTI_KYC=$WPAY_CUSTOMER_KEY_MULTI_KYC \
      --env WPAY_MERCHANT_ID_MULTI_KYC=$WPAY_MERCHANT_ID_MULTI_KYC \
      --include-tags pay \
      --test-output-dir /tmp/maestro-output \
      .maestro/

Fix this →

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the E2E Pay CI workflow to pin all WalletConnect/actions Maestro action usages (pay test flow sync, CLI setup, and Permit2 reset) to a newer commit SHA that includes the updated Maestro flows for the redesigned /collect KYC webview.

Changes:

  • Bumped WalletConnect/actions/maestro/pay-tests to ddc7c0f9acd8b1576e9a78f0ef67f4b8fbb36db2.
  • Bumped WalletConnect/actions/maestro/setup to ddc7c0f9acd8b1576e9a78f0ef67f4b8fbb36db2.
  • Bumped WalletConnect/actions/maestro/permit2-reset to ddc7c0f9acd8b1576e9a78f0ef67f4b8fbb36db2.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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