ci(mitmproxy): test mitmproxy #14075
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 4 Skipped Deployments
|
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
❌ Action Required: Monitored Files ChangedThe following files in monitored folders have been modified:
Action Required: Please rebase your branch against git rebase origin/develop |
There was a problem hiding this comment.
Pull request overview
This PR updates the mobile mock test GitHub Actions workflow to add mitmproxy logging for iOS detox tests and to normalize some workflow syntax/formatting.
Changes:
- Standardizes YAML quoting and
runs-on/needsarray formatting in the reusable mobile test workflow. - Adds a
yaegashi/mitmproxy-logger-action@v1step to the iOS detox job to start a local mitmproxy instance with CA cert installation and proxy env vars enabled. - Leaves existing build, detox, and reporting logic intact while wiring mitmproxy in right before the iOS e2e test step.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| listen-port: "8080" | ||
| install-cacert: true | ||
| set-envvars: true | ||
| passphrase: "remove-me" |
There was a problem hiding this comment.
The passphrase value for yaegashi/mitmproxy-logger-action is hardcoded to a trivial string, which is risky given that mitmproxy will capture potentially sensitive network traffic; this effectively makes the encrypted capture easy to decrypt by anyone with access to the workflow definition. Consider sourcing this passphrase from a GitHub secret (or similar secure configuration) instead of checking it into the workflow file.
| passphrase: "remove-me" | |
| passphrase: ${{ secrets.MITMPROXY_PASSPHRASE }} |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| enabled: true | ||
| listen-host: "127.0.0.1" | ||
| listen-port: "8080" | ||
| passphrase: "remove-me" |
There was a problem hiding this comment.
The mitmproxy step is using a hard-coded passphrase value ("remove-me"), which both weakens the protection of any captured traffic/logs and introduces a secret-like value directly into the workflow definition. It would be safer to either let the action generate its own passphrase or source a strong passphrase from GitHub Actions secrets or an environment variable rather than committing a placeholder string here.
| passphrase: "remove-me" | |
| passphrase: ${{ secrets.MITMPROXY_PASSPHRASE }} |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # report-bundle-sizes: | ||
| # name: "Build Mobile > Report Bundle Sizes" | ||
| # runs-on: [ubuntu-22.04] | ||
| # if: needs.build-ios.outputs.js-bundle-size != '' && needs.build-android.outputs.js-bundle-size != '' | ||
| # needs: [build-ios, build-android] |
There was a problem hiding this comment.
report-bundle-sizes still declares needs: [build-ios, build-android], but the build-ios job has been fully commented out above. GitHub Actions will reject this workflow because needs references a non-existent job; either restore build-ios or remove it from needs and adjust the if condition and bundle size logic accordingly.
| # needs: [determine-builds, detox-tests-ios] | ||
| # runs-on: ubuntu-24.04 | ||
| # if: ${{ !cancelled() && (needs.detox-tests-ios.result == 'success' || needs.detox-tests-ios.result == 'failure') && inputs.max-shards > 1 }} |
There was a problem hiding this comment.
merge-ios-timings depends on detox-tests-ios in both needs and the if condition, but the detox-tests-ios job is now fully commented out. This will cause the workflow to fail validation (unknown job in needs) and makes the iOS timing merge step impossible to run; either re-enable detox-tests-ios or remove it from merge-ios-timings and adjust the condition/logic.
| # needs: [determine-builds, detox-tests-ios] | |
| # runs-on: ubuntu-24.04 | |
| # if: ${{ !cancelled() && (needs.detox-tests-ios.result == 'success' || needs.detox-tests-ios.result == 'failure') && inputs.max-shards > 1 }} | |
| needs: [determine-builds] | |
| runs-on: ubuntu-24.04 | |
| if: ${{ !cancelled() && inputs.max-shards > 1 }} |
| ios_native_exists: ${{ steps.check-ios-native.outputs.cache-hit }} | ||
| ios_js_exists: ${{ steps.check-ios-js.outputs.cache-hit }} |
There was a problem hiding this comment.
The determine-builds outputs ios_native_exists and ios_js_exists still reference steps.check-ios-native and steps.check-ios-js, but those steps are now commented out. As a result these outputs will never be set from a real cache lookup, which breaks any downstream logic relying on them and should be updated (or removed) in line with the disabled iOS cache checks.
| ios_native_exists: ${{ steps.check-ios-native.outputs.cache-hit }} | |
| ios_js_exists: ${{ steps.check-ios-js.outputs.cache-hit }} | |
| ios_native_exists: 'false' | |
| ios_js_exists: 'false' |
| # test-pod-lockfile: | ||
| # name: "Test Pod Lockfile" | ||
| # runs-on: [macOS, ARM64] | ||
| # needs: [determine-builds] | ||
| # if: needs.determine-builds.outputs.ios_native_exists == 'true' && inputs.skip-pod-checks != 'true' |
There was a problem hiding this comment.
The test-pod-lockfile job's if condition depends on needs.determine-builds.outputs.ios_native_exists == 'true', but the ios_native_exists output is now wired to a commented-out check-ios-native step and will never become 'true'. This effectively disables the pod lockfile test even when an iOS native build cache exists; consider either updating how ios_native_exists is computed or explicitly deciding (and documenting) that this job should be skipped.
| enabled: true | ||
| listen-host: "127.0.0.1" | ||
| listen-port: "8080" | ||
| passphrase: "remove-me" |
There was a problem hiding this comment.
The mitmproxy step hardcodes a passphrase: "remove-me" in the workflow, which is effectively a credential checked into the repo. For better security hygiene, this passphrase should be managed via GitHub Actions secrets or other secure configuration, even if it is only used for CI test traffic.
| passphrase: "remove-me" | |
| passphrase: "${{ secrets.MITMPROXY_PASSPHRASE }}" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mv ${{steps.mitmproxy.outputs.cacert-path}} mitmproxy.pem | ||
| adb root | ||
| adb remount | ||
| adb push mitmproxy.pem /sdcard/ | ||
| adb shell "mv /sdcard/mitmproxy.pem /system/etc/security/cacerts/" | ||
| adb shell "chmod 644 /system/etc/security/cacerts/mitmproxy.pem" |
There was a problem hiding this comment.
The mitmproxy CA certificate is being pushed into /system/etc/security/cacerts as mitmproxy.pem, but Android’s system trust store expects CA files to use the subject-hash-based filename convention (e.g. <hash>.0) rather than arbitrary names, otherwise the cert will be ignored. To ensure the emulator actually trusts mitmproxy, generate the proper hashed filename (using openssl x509 -subject_hash_old or an equivalent mechanism) before pushing/moving the cert into the system CA directory instead of renaming it to mitmproxy.pem.
| mv ${{steps.mitmproxy.outputs.cacert-path}} mitmproxy.pem | |
| adb root | |
| adb remount | |
| adb push mitmproxy.pem /sdcard/ | |
| adb shell "mv /sdcard/mitmproxy.pem /system/etc/security/cacerts/" | |
| adb shell "chmod 644 /system/etc/security/cacerts/mitmproxy.pem" | |
| CACERT_PATH=${{ steps.mitmproxy.outputs.cacert-path }} | |
| HASH=$(openssl x509 -inform PEM -subject_hash_old -in "$CACERT_PATH" | head -n 1) | |
| CERT_FILENAME="${HASH}.0" | |
| cp "$CACERT_PATH" "$CERT_FILENAME" | |
| adb root | |
| adb remount | |
| adb push "$CERT_FILENAME" /sdcard/ | |
| adb shell "mv /sdcard/$CERT_FILENAME /system/etc/security/cacerts/" | |
| adb shell "chmod 644 /system/etc/security/cacerts/$CERT_FILENAME" |
| # build-ios: | ||
| # name: "iOS Build" | ||
| # needs: [determine-builds] | ||
| # uses: LedgerHQ/ledger-live/.github/workflows/test-mobile-build-ios-reusable.yml@develop | ||
| # with: |
There was a problem hiding this comment.
Commenting out the iOS build/test and related timing/bundle-size/pod-lockfile jobs (build-ios, detox-tests-ios, merge-ios-timings, report-bundle-sizes, test-pod-lockfile) means this reusable workflow now only exercises Android paths when invoked from the main PR/push workflows, dropping iOS CI coverage and size checks entirely. If the intent is just to experiment with mitmproxy for Android, it would be safer to either gate these changes behind an input flag or keep the iOS jobs enabled so that existing iOS build/test guarantees are not lost when this workflow is used in build-and-test-pr.yml / build-and-test-push.yml.
| run: | | ||
| mv ${{steps.mitmproxy.outputs.cacert-path}} mitmproxy.pem | ||
| adb root | ||
| adb remount | ||
| adb push mitmproxy.pem /sdcard/ | ||
| adb shell "mv /sdcard/mitmproxy.pem /system/etc/security/cacerts/" | ||
| adb shell "chmod 644 /system/etc/security/cacerts/mitmproxy.pem" |
There was a problem hiding this comment.
This step runs ADB commands even when no emulator is running: the previous reactivecircus/android-emulator-runner step both runs only when steps.detox-avd.outputs.cache-hit != 'true' and tears down the emulator once its script completes, while this step has no if condition at all. As a result, on cache hits there is never an emulator started in this job, and even on cache misses the emulator is already shut down by the time this runs, so adb root/adb remount/adb push are expected to fail. To reliably provision the cert, these ADB commands should be executed while the emulator is still alive (e.g. inside the script invoked by android-emulator-runner or in a separate android-emulator-runner step with its own script), and/or this step should share the same conditional as the AVD creation step.
| run: | | |
| mv ${{steps.mitmproxy.outputs.cacert-path}} mitmproxy.pem | |
| adb root | |
| adb remount | |
| adb push mitmproxy.pem /sdcard/ | |
| adb shell "mv /sdcard/mitmproxy.pem /system/etc/security/cacerts/" | |
| adb shell "chmod 644 /system/etc/security/cacerts/mitmproxy.pem" | |
| uses: reactivecircus/android-emulator-runner@v2 | |
| with: | |
| api-level: ${{ env.AVD_API }} | |
| arch: ${{ env.AVD_ARCH }} | |
| profile: ${{ env.AVD_PROFILE }} | |
| target: ${{ env.AVD_TARGET }} | |
| avd-name: ${{ env.AVD_NAME }} | |
| cores: ${{ env.AVD_CORES }} | |
| ram-size: ${{ env.AVD_RAM_SIZE }} | |
| disable-linux-hw-accel: false | |
| emulator-options: ${{ env.AVD_OPTIONS }} -writable-system | |
| script: | | |
| mv ${{steps.mitmproxy.outputs.cacert-path}} mitmproxy.pem | |
| adb root | |
| adb remount | |
| adb push mitmproxy.pem /sdcard/ | |
| adb shell "mv /sdcard/mitmproxy.pem /system/etc/security/cacerts/" | |
| adb shell "chmod 644 /system/etc/security/cacerts/mitmproxy.pem" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| [tools] | ||
| python = "{{ get_env(name='PYTHON_VERSION', default='3.11') }}" | ||
| mitmproxy = "{{ get_env(name='MIMTPROXY_VERSION', default='12.2.1') }}" |
There was a problem hiding this comment.
The mise_toml configuration defines mitmproxy = "{{ get_env(name='MIMTPROXY_VERSION', default='12.2.1') }}", but the environment variable name appears to be misspelled (MIMTPROXY_VERSION instead of MITMPROXY_VERSION). As written, any MITMPROXY_VERSION env var will be ignored and mitmproxy will always resolve to the hard-coded default, so the intended version override mechanism will not work.
| mitmproxy = "{{ get_env(name='MIMTPROXY_VERSION', default='12.2.1') }}" | |
| mitmproxy = "{{ get_env(name='MITMPROXY_VERSION', default='12.2.1') }}" |
| adb root | ||
| adb remount | ||
| adb push ~/.mitmproxy/mitmproxy-ca.pem /sdcard/ | ||
| adb shell "mv /sdcard/mitmproxy-ca.pem /system/etc/security/cacerts/" | ||
| adb shell "chmod 644 /system/etc/security/cacerts/mitmproxy-ca.pem" |
There was a problem hiding this comment.
The mitmproxy CA certificate is pushed into /system/etc/security/cacerts/ as mitmproxy-ca.pem, but Android's system trust store expects PEM-encoded certificates whose filenames are the OpenSSL subject hash with a numeric suffix (for example <hash>.0). Without renaming the file to the expected hash-based name, the emulator is unlikely to recognize and trust this CA, defeating the purpose of provisioning mitmproxy; consider computing the hash-based filename (or using c_rehash-style logic) and renaming the certificate accordingly.
| adb root | |
| adb remount | |
| adb push ~/.mitmproxy/mitmproxy-ca.pem /sdcard/ | |
| adb shell "mv /sdcard/mitmproxy-ca.pem /system/etc/security/cacerts/" | |
| adb shell "chmod 644 /system/etc/security/cacerts/mitmproxy-ca.pem" | |
| set -euo pipefail | |
| CERT_PATH="$HOME/.mitmproxy/mitmproxy-ca.pem" | |
| # Compute the OpenSSL subject hash used by Android for CA filenames. | |
| # Try the "old" hash format first (OpenSSL 1.0-style), then fall back to the default. | |
| HASH="$(openssl x509 -in "$CERT_PATH" -subject_hash_old -noout 2>/dev/null || openssl x509 -in "$CERT_PATH" -subject_hash -noout)" | |
| DEST_NAME="${HASH}.0" | |
| adb root | |
| adb remount | |
| adb push "$CERT_PATH" "/system/etc/security/cacerts/${DEST_NAME}" | |
| adb shell "chmod 644 /system/etc/security/cacerts/${DEST_NAME}" |
| # build-ios: | ||
| # name: "iOS Build" | ||
| # needs: [determine-builds] | ||
| # uses: LedgerHQ/ledger-live/.github/workflows/test-mobile-build-ios-reusable.yml@develop | ||
| # with: | ||
| # ref: ${{ inputs.ref || github.sha }} | ||
| # macos-specificity-runner-label: ${{ inputs.macos-specificity-runner-label }} | ||
| # disable-turbo-cache: ${{ inputs.disable-turbo-cache || false }} | ||
| # build-ios-js: ${{ needs.determine-builds.outputs.ios_js_exists == 'false' }} | ||
| # build-ios-native: ${{ needs.determine-builds.outputs.ios_native_exists == 'false' }} | ||
| # ios-native-cache-key: ${{ needs.determine-builds.outputs.ios_native_key }} | ||
| # ios-js-cache-key: ${{ needs.determine-builds.outputs.ios_js_key }} | ||
| # secrets: inherit | ||
|
|
There was a problem hiding this comment.
Several entire jobs (e.g. build-ios, detox-tests-ios, report-bundle-sizes, merge-*-timings, test-pod-lockfile) are now fully commented out, leaving large blocks of dead configuration in this reusable workflow. If these paths are intentionally disabled for mitmproxy experiments, consider either removing them or gating them behind an input flag (or if: condition) rather than long-lived commented code, to keep the workflow easier to maintain and reduce the risk of these stale blocks drifting from the active configuration.
| # build-ios: | |
| # name: "iOS Build" | |
| # needs: [determine-builds] | |
| # uses: LedgerHQ/ledger-live/.github/workflows/test-mobile-build-ios-reusable.yml@develop | |
| # with: | |
| # ref: ${{ inputs.ref || github.sha }} | |
| # macos-specificity-runner-label: ${{ inputs.macos-specificity-runner-label }} | |
| # disable-turbo-cache: ${{ inputs.disable-turbo-cache || false }} | |
| # build-ios-js: ${{ needs.determine-builds.outputs.ios_js_exists == 'false' }} | |
| # build-ios-native: ${{ needs.determine-builds.outputs.ios_native_exists == 'false' }} | |
| # ios-native-cache-key: ${{ needs.determine-builds.outputs.ios_native_key }} | |
| # ios-js-cache-key: ${{ needs.determine-builds.outputs.ios_js_key }} | |
| # secrets: inherit |
✅ Checklist
npx changesetwas attached.📝 Description
Replace this text by a clear and concise description of what this pull request is about and why it is needed. Be sure to explain the problem you're addressing and the solution you're proposing.
For libraries, you can add a code sample of how to use it.
For bug fixes, you can explain the previous behaviour and how it was fixed.
In case of visual features, please attach screenshots or video recordings to demonstrate the changes.
❓ Context
🧐 Checklist for the PR Reviewers