ci: enable PR Preview workflow with security hardening (Phase 1)#2772
Open
piyalbasu wants to merge 16 commits into
Open
ci: enable PR Preview workflow with security hardening (Phase 1)#2772piyalbasu wants to merge 16 commits into
piyalbasu wants to merge 16 commits into
Conversation
Activate the prPreview.yml workflow (move from workflow-drafts/ to
workflows/) and apply the Phase 1 security fixes from the multi-agent
review documented in pr-preview-workflow-security-review.md:
- Use draft GitHub Releases (not prereleases) so the artifact is scoped
to SDF collaborators only; non-SDF GitHub users get 404
- Per-job permissions (workflow-level permissions: {}); contents: write
is granted only where release operations need it
- Separate concurrency groups for build vs cleanup so a PR close cannot
cancel an in-flight build mid-publish
- Strip source maps (find ./extension/build -name "*.map" -delete)
before zipping; background.min.js.map etc. no longer ship in the
public-ish artifact
- Replace jossef/action-set-json-field, restackio/update-json-file-action,
and montudor/action-zip with inline jq + zip shell steps; drops three
third-party supply-chain dependencies (one was a no-op anyway)
- Author-association gate (MEMBER/OWNER only) on the publish step plus
the fork-guard if: as defense-in-depth; verified empirically that
fork PRs do not see workflow secrets
- Forbidden-triggers comment block at top of file documents the durable
invariant against issue_comment / pull_request_target etc., which is
the class of vulnerability that hit freighter-mobile e2e workflows
- Manifest hardening: assert no top-level `key` field, rewrite Firefox
gecko.id to a preview-specific UUID, rewrite manifest name and
version_name to "Freighter PR Preview #N"
- Sticky PR comment with download link + install instructions, plus a
fresh-profile reviewer protocol note
Design doc: Fullstack PR Preview Flow (Phase 1 row "Extension workflow"
and "Workflow security hardening (extension)").
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Phase 1 security review flagged source maps in the public preview
zip as a confidentiality issue ("reveals wallet/key-handling source").
That threat model doesn't apply here — stellar/freighter is a public
open-source repo, so the TypeScript source is already at
github.com/stellar/freighter and source maps in the preview zip leak
nothing that isn't already inspectable.
The original review file has been updated to retract the M2 finding;
the design doc's Phase 1 hardening row has been updated to drop the
recommendation.
If we ever ship a closed-source build channel, revisit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous wording ("use a fresh browser profile with no real keys
imported") didn't match the actual risk. The threat isn't that an
existing wallet "contaminates" the preview profile — it's that the
preview build is mid-review code that may contain unfixed bugs in
signing, fee calculation, transaction construction, etc. Reframe to
that.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Match the release --notes wording to the sticky PR comment: focus on review-stage code risk, not on the awkward "do not install" phrasing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
piyalbasu
added
the
Fullstack PR Preview
6 tasks
The literal block scalar (|) preserves a trailing newline that GitHub Actions' expression evaluator does not handle gracefully — the if: silently evaluates to false even when all three conditions are true. Empirical evidence: first PR run on this branch showed both build and cleanup jobs as 'skipped' despite head.repo.full_name matching, action being 'opened', and author_association being MEMBER (verified via the REST API on the PR). Folded block scalar with strip chomping (>-) collapses the multi-line expression into a single-line string with no trailing whitespace, which the evaluator parses correctly. Standard pattern for multi-line if: in production workflows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The webhook event payload's author_association field only reflects PUBLIC org membership. SDF members with private memberships show as CONTRIBUTOR rather than MEMBER in the workflow runtime context, which caused the gate to lock out legitimate SDF contributors. Diagnosis via temporary debug step: github.event.pull_request.author_association returned CONTRIBUTOR for my (piyalbasu) PR despite the REST API correctly returning MEMBER. Root cause: my stellar org membership is private (the default for org members at GitHub orgs that don't enforce public). Fix: rely on the two remaining gates, which match the pattern other freighter-mobile workflows use (e.g., ios-e2e.yml): 1. Job-level if: fork-guard (head.repo.full_name == github.repository) 2. Platform-level fork-PR secret-withholding (read-only token for forks) Plus org-level "Require approval for outside collaborators" catches non-SDF fork PRs before they can even queue. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-8fca4076c3d4a6701a66 (SDF collaborators only — install instructions in the release description) |
Move the verbose install instructions, warnings, and metadata from the sticky PR comment to the release description. The PR comment is now a single line pointing at the release. A reviewer landing on the release page gets all the context they need (link to source PR, commit, backend target, install steps, review-stage caution). Motivation: the prior comment was 10+ lines of markdown, which became noisy when stacked alongside other bot comments on a busy PR. The release description is the natural place for install info anyway — that's where the asset lives. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same fix as stellar/freighter-mobile — bash command substitution tokenizes single quotes inside the heredoc body, breaking on prose apostrophes. Use --notes-file to bypass the parser quirk. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Enables the PR Preview GitHub Actions workflow for extension PRs by promoting the previously drafted workflow into .github/workflows/, with additional security hardening and release/comment automation to distribute internal-only preview artifacts.
Changes:
- Added
.github/workflows/prPreview.ymlto build PR-specific extension bundles and publish them as draft GitHub Releases with a sticky PR comment. - Added manifest guards/rewrites for preview identity and Chromium/Firefox isolation.
- Removed the old commented-out draft workflow from
.github/workflow-drafts/.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| .github/workflows/prPreview.yml | New PR Preview workflow to build, draft-release, and comment preview install links with added security checks. |
| .github/workflow-drafts/prPreview.yml | Removed obsolete draft workflow stub now that the real workflow is enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
piyalbasu
added a commit
to stellar/freighter-mobile
that referenced
this pull request
May 12, 2026
Five fixes from Copilot's review on PR #868 plus the same fixes applied in stellar/freighter#2772 for consistency: 1. Concurrency: collapse build + cleanup into a single workflow-level concurrency group with cancel-in-progress: true. Cross-applied from stellar/freighter's review. 2. URL capture: stop capturing stdout from `gh release create`; query the URL via `gh release view --json url`. Cross-applied. 3. Add --target ${PR_HEAD_SHA} to gh release create. Copilot flagged this one on this PR. 4. --paginate on the sticky-comment search. Copilot flagged this one on this PR. 5. find -maxdepth: moved before other predicates. BSD/macOS find can silently ignore -maxdepth when it's placed after -name/-type, which would let the .app discovery fail on the macos runner. Copilot flagged this. The Release-vs-Debug configuration question Copilot raised: kept as Release (intentional, as documented inline). Debug expects Metro at runtime and would force reviewers to maintain a local checkout to run the .app — defeats the purpose of a standalone preview. PR description has been updated to make this explicit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five fixes from Copilot's review on PR #2772: 1. Concurrency: collapse build + cleanup into a single workflow-level concurrency group with cancel-in-progress: true. Previously they used separate groups, which allowed a close-event cleanup to run while the build was still in-flight, producing an orphaned release after cleanup finished. Shared group means close cancels the in-flight build before cleanup deletes. 2. URL capture: stop capturing stdout from `gh release create` (can include progress lines); query the URL with `gh release view --json url --jq '.url'` after creation. 3. Add --target ${PR_HEAD_SHA} to gh release create so if the draft is ever manually published, the resulting git tag points at the PR's HEAD commit, not master. 4. --paginate on the sticky-comment search so the marker can be found on PRs with >30 comments. 5. Move INDEXER_URL / INDEXER_V2_URL from workflow-level env to the build job's env. Defense-in-depth: a future job added without the same fork-guard if: won't inherit the secrets. Plus a small fix: PREVIEW_NAME now includes the "#" prefix to match the release title. Copilot's source-map comment is intentionally not addressed — see the PR description and pr-preview-workflow-security-review.md (M2 retracted): stellar/freighter is a public repo, the TypeScript source is already at github.com/stellar/freighter, so source maps in the preview zip leak nothing that isn't already public. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
leofelix077
approved these changes
May 14, 2026
The previous wiring used: INDEXER_URL = secrets.INDEXER_URL (prod v1) INDEXER_V2_URL = secrets.INDEXER_V2_BETA_URL (beta v2) A mismatched pair — v1 prod + v2 beta — and the v1 staging hostname isn't publicly resolvable anyway (no public DNS for freighter-backend-stg.stellar.org per the kube manifests). Switch both to prod: INDEXER_URL = secrets.INDEXER_URL (prod v1) INDEXER_V2_URL = secrets.INDEXER_V2_URL (prod v2) freighter-backend is a read-side indexer (balances, assets, history). The wallet submits transactions directly to Horizon/RPC, so the backend choice here doesn't affect write paths. Read-only prod traffic is acceptable for preview review. Release-notes "Backend: staging" updated to "production (read-only indexer; wallet writes go direct to Horizon/RPC)" so reviewers know what they're hitting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts the V2 URL back to INDEXER_V2_BETA_URL from INDEXER_V2_URL. The previous "both prod" wiring was a misread of the user's intent — V2 beta is publicly reachable and is the right target for preview- stage testing. V1 stays on prod because V1 beta is not publicly resolvable. Release-notes wording updated to "V1 production, V2 beta". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Findings from multi-agent adversarial review of #2772: - persist-credentials:false on actions/checkout. Default leaves a GITHUB_TOKEN auth header in .git/config for the rest of the job; any subsequent code execution (yarn lifecycle scripts, build scripts) could `git push` via those persisted creds. `gh` uses GH_TOKEN env separately so this doesn't affect the release flow. - Re-assert no top-level `key` on the BUILT manifest, post-build and pre-zip. The pre-existing source-manifest check catches a hand- edited `key`, but a webpack plugin / postinstall script / transitive dep could still inject one into ./extension/build/manifest.json after the source assertion runs. Without the post-build re-check, a `key`-bearing artifact would ship and the preview install would collide with Web Store Freighter's Chromium extension ID. - Replace `gh release delete ... || true` with check-then-delete in both the pre-create step and the cleanup job. `|| true` swallows transient API errors (network, 422 tag-conflict) alongside the not-found case, letting a stale tag survive into the next `gh release create` call (which would silently reuse the existing tag, pointing it at an old commit). - Drop `--target ${PR_HEAD_SHA}` from `gh release create`. With the flag, any account that can click "Publish release" on a preview draft mints an official-looking tag pointing at unreviewed PR-head code — branch protection rules do not extend to release publication. Without the flag, a published draft falls back to master HEAD (reviewed code). The PR's HEAD commit is still recorded in the release notes for traceability. Verifications that came back clean (no code change): all action SHA pins match the upstream tag SHAs claimed in comments. Residual risk accepted: yarn install lifecycle scripts run with INDEXER_URL / INDEXER_V2_BETA_URL and GITHUB_TOKEN. The secret set is deliberately low-value (backend URLs, not signing material) and any attacker with repo write access has parallel paths to the same outcome; closing this class requires per-PR-push approval friction not warranted at the current secret level. Re-evaluate if Apple Connect / Sentry / signing tokens ever enter the secret set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The check-then-delete logic added in the prior commit (replacing `|| true`) exposed a pre-existing bug masked by the old code: `gh release delete --cleanup-tag` fails with HTTP 422 "Reference does not exist (refs/tags/pr-preview-N)" when the release is a draft, because drafts don't create the git tag until publish. The previous `|| true` silently swallowed this 422 every run (along with real errors we now correctly surface). The fix: query `isDraft` and branch on it — drop `--cleanup-tag` for drafts, keep it for the (defensive) published-release case so a manually- published draft's tag still gets cleaned up by the cleanup job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
piyalbasu
added a commit
to stellar/freighter-mobile
that referenced
this pull request
May 15, 2026
* ci: add iOS Simulator PR Preview workflow (Phase 1) Add a new prPreviewIos.yml workflow that builds an unsigned iOS Simulator .app on every PR opened by an SDF org member, attaches it to a draft GitHub Release on stellar/freighter-mobile, and posts a sticky PR comment with download + install instructions. Phase 1 deliberately uses Simulator builds (not TestFlight) because Simulator builds need NO Apple Developer credentials — no App Store Connect API key, no Fastlane Match cert, no provisioning profile. This eliminates the credential surface that would otherwise be the largest attack target if the workflow were ever compromised. Physical device testing (TestFlight) is deferred to Phase 3 where it pairs with the Tailscale rollout (sandbox URLs are not reachable from iOS devices without an OS-level VPN, so TestFlight against sandbox is not viable without Tailscale anyway). Security model mirrors the extension prPreview.yml: - Trigger is pull_request only — never pull_request_target, issue_comment, or workflow_run. Forbidden-triggers invariant documented at the top of the YAML; explicit reference to the Q2 2026 e2e RCE class that this gate prevents. - Job-level if: gate combining fork-guard + author_association in ['MEMBER', 'OWNER'], evaluated before secrets are injected. Verified empirically (stellar/freighter#2760, since closed) that fork PRs do not see workflow secrets. - Workflow-level permissions: {}; contents: write and pull-requests: write granted per-job only where needed. - Separate concurrency groups for build vs cleanup so a PR close cannot cancel an in-flight build mid-publish. - xcodebuild target is iphonesimulator SDK with CODE_SIGNING_REQUIRED=NO, arm64 + x86_64 universal. The resulting .app is unsigned and runs only in iOS Simulator. - Only one repo Secret referenced: WALLET_KIT_PROJECT_ID_DEV (mapped into both PROD and DEV WalletKit env slots so production WalletKit is unreachable from preview builds). Telemetry secrets (Sentry, Amplitude) are stubbed with literal strings. - Backend URLs all point at staging (Phase 2 will switch to freighter-config sandbox URL injection). - Draft GitHub Release is created with --draft (not --prerelease) so it is visible to SDF collaborators only; non-SDF GitHub users get 404 on the URL. Visibility model verified during design on stellar/freighter; mirrors here because both repos are in the same org with the same Read-base permission settings. Cocoapods cache mirrors ios-e2e.yml's broader path set + restore-keys fallback so PR builds get a warm cache on Podfile.lock changes. Design doc: Fullstack PR Preview Flow (Phase 1 row "iOS Simulator .app build", § Security, § Pathways → Mobile → iOS). Related issue: #860 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: reframe preview-install warning to focus on review-stage risk The previous wording ("use a fresh wallet — never enter real keys") didn't match the actual risk. The threat isn't that an existing wallet "contaminates" the preview build — it's that the preview is mid-review code that may contain unfixed bugs in signing, fee calculation, transaction construction, etc. Reframe to that. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: use folded block scalar (>-) for multi-line if: Same fix as stellar/freighter — the literal block scalar (|) for the job-level if: causes GitHub Actions to silently skip both jobs even when all conditions evaluate to true. Switching to >- (folded, strip trailing newline) lets the expression evaluator parse correctly. Affects both the build and cleanup jobs in this workflow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: temporary debug job to diagnose skipped build TEMPORARY — remove once we figure out why the build job's if: skips. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: drop author_association gate + remove debug job Same fix as stellar/freighter — the webhook event payload's author_association field only reflects PUBLIC org membership, so SDF members with private memberships were locked out (showing as CONTRIBUTOR instead of MEMBER in the workflow runtime). Diagnosed via the temporary debug-context job in the previous commit, which showed author_association=CONTRIBUTOR for piyalbasu despite the REST API returning MEMBER. Debug job removed in this commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: work around build-scrypt-16kb NDK detection bug Override ANDROID_NDK_HOME / ANDROID_NDK_ROOT to point at the parent .../ndk/ directory before yarn install + yarn postinstall, so scripts/build-scrypt-16kb-aligned.js can readdirSync version subdirs and pick the latest. The macos-26-xlarge runner image's default for those env vars points at a specific version subdir (e.g. .../ndk/27.3.13750724), which the script can't handle. This is a workaround. The proper fix is to patch build-scrypt-16kb-aligned.js to also handle a version-pinned NDK path (use it directly if it looks like a valid NDK version dir, instead of trying to enumerate sub-versions). That patch is out of scope here. Other repo workflows (ios.yml, ios-e2e.yml) will hit the same error on their next runner-image refresh and should adopt the script fix once it lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: move install instructions to release description, slim PR comment Same change as stellar/freighter — move the verbose install steps and warnings from the sticky PR comment into the release --notes. PR comment is now one line pointing at the release. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: set SENTRY_DISABLE_AUTO_UPLOAD=true for xcodebuild step The Xcode project's "Upload Debug Symbols" build phase calls sentry-cli to upload source maps. Preview builds intentionally disable Sentry, but the build phase still attempts the upload and fails: error: sentry-cli - To disable source maps auto upload, set SENTRY_DISABLE_AUTO_UPLOAD=true in your environment variables. error: An organization ID or slug is required (provide with --org) ** BUILD FAILED ** Setting SENTRY_DISABLE_AUTO_UPLOAD=true on the xcodebuild step tells the Sentry build phase to skip the upload entirely. Matches the pattern in ios-e2e.yml (line 152) which the Phase 1 code review flagged but I initially dismissed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: write release notes to file instead of $(cat <<EOF) capture Bash command substitution ($(...)) tokenizes single quotes inside a heredoc body, so an apostrophe in prose ("haven't been caught yet") breaks the parse with: /tmp/...sh: line 16: unexpected EOF while looking for matching ' Switch to `cat > file` + `gh release create --notes-file` which avoids the command-substitution-with-heredoc parser quirk entirely. Also rephrased the one apostrophe ("haven't" → "have not") as belt and suspenders in case anything else tries to capture the body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: build Release configuration so JS bundle embeds in the .app The previous Debug configuration expected Metro to serve JS at runtime. That forced reviewers to keep a local freighter-mobile checkout + run Metro to launch the preview .app, which defeated the entire point of a self-contained preview build: Unable to resolve module @exodus/patch-broken-hermes-typed-arrays from /Users/<reviewer>/Stellar/freighter-mobile/index.js: ... Release configuration triggers React Native's "Bundle React Native code and images" Xcode build phase, which runs metro at BUILD time and embeds the JS bundle directly into the .app. Reviewers can drag the .app into Simulator and run it without any local setup. Matches the existing fastlane :dev lane (fastlane/Fastfile:128-140), which already uses configuration: "Release". This workflow should have mirrored that pattern from the start. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: address Copilot review feedback Five fixes from Copilot's review on PR #868 plus the same fixes applied in stellar/freighter#2772 for consistency: 1. Concurrency: collapse build + cleanup into a single workflow-level concurrency group with cancel-in-progress: true. Cross-applied from stellar/freighter's review. 2. URL capture: stop capturing stdout from `gh release create`; query the URL via `gh release view --json url`. Cross-applied. 3. Add --target ${PR_HEAD_SHA} to gh release create. Copilot flagged this one on this PR. 4. --paginate on the sticky-comment search. Copilot flagged this one on this PR. 5. find -maxdepth: moved before other predicates. BSD/macOS find can silently ignore -maxdepth when it's placed after -name/-type, which would let the .app discovery fail on the macos runner. Copilot flagged this. The Release-vs-Debug configuration question Copilot raised: kept as Release (intentional, as documented inline). Debug expects Metro at runtime and would force reviewers to maintain a local checkout to run the .app — defeats the purpose of a standalone preview. PR description has been updated to make this explicit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: ad-hoc sign the simulator build so keychain entitlements take effect Reviewer-reported failure: opening the preview in iOS Simulator produced "Failed to set key ${id}" errors on every wallet save (ReactNativeKeychainFacade.ts:108). Root cause: the freighter-mobile-dev.entitlements file declares keychain-access-groups: org.stellar.freighterdev. iOS only honors entitlements when the binary is signed (even ad-hoc). The previous unsigned simulator build had no entitlements applied, so the OS denied writes to the named keychain access group, and react-native-keychain surfaced the denial as "failed to set key". Switch to ad-hoc signing (CODE_SIGN_IDENTITY="-"): - Doesn't require a real cert, provisioning profile, or developer team — works in CI with no credential setup - Embeds the entitlements file into the binary so the Simulator honors keychain access group permissions - Supported for iphonesimulator SDK builds — this is the standard pattern for unsigned-but-entitled simulator builds Other CODE_SIGN_* flags are explicit to avoid Xcode falling back to real-cert lookup that fails in CI: CODE_SIGN_STYLE=Manual — don't try automatic provisioning DEVELOPMENT_TEAM="" — no team needed for ad-hoc PROVISIONING_PROFILE_SPECIFIER="" — no profile needed for ad-hoc Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: drop CODE_SIGN_ENTITLEMENTS from xcodebuild flags The previous commit added CODE_SIGN_ENTITLEMENTS=ios/freighter-mobile/ freighter-mobile-dev.entitlements at the xcodebuild command line. That override applies to EVERY target in the workspace, including the Pods/* projects, which try to resolve the relative path from their own source directory and fail: error: Build input file cannot be found: '.../ios/Pods/ios/freighter-mobile/freighter-mobile-dev.entitlements' ** BUILD FAILED ** (in target 'react-native-cameraroll-RNCameraRollPrivacyInfo' from project 'Pods') The main app target already declares CODE_SIGN_ENTITLEMENTS in its .pbxproj — we don't need to override it from the command line. Removing the flag lets each target use the entitlements file it's configured for (or, for Pods, no entitlements at all). CODE_SIGN_IDENTITY="-" still applies workspace-wide, which is fine: Pods targets get ad-hoc signed too, and they don't need entitlements. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: point preview at prod freighter-backend (not staging) The v1 staging hostname (freighter-backend-stg.stellar.org) is not publicly resolvable — per the kube manifests, v1 stg only has the kube-internal freighter-backend-stg.kube001-dev.services.stellar-ops.com ingress, which requires sshuttle. NXDOMAIN on the public hostname. Result: reviewer-reported failure loading balances in the iOS Simulator preview. Fix: point all FREIGHTER_BACKEND_V{1,2}_*_URL slots at the PROD variants (vars.FREIGHTER_BACKEND_V{1,2}_PROD_URL), which DO have public DNS. The backend is a read-side indexer (balances, assets, history); the wallet submits txs directly to Horizon/RPC, so this doesn't affect write paths. Release-notes "Backend: staging" updated to "production (read-only indexer; wallet writes go direct to Horizon/RPC)" so reviewers know what they're hitting. When v1 stg gets a public ingress (Ops follow-up), swap back to the _STG_URL variants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: V1 prod, V2 staging (V2 back to STG) Reverts the V2 URL slot back to vars.FREIGHTER_BACKEND_V2_STG_URL (freighter-backend-v2-stg.stellar.org), which IS publicly resolvable. The previous "both PROD" wiring was a misread of the user's intent — V2 staging is the right preview-stage target. V1 stays on prod because V1 staging has no public DNS. Release-notes wording updated to "V1 production, V2 staging/beta". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: SHA-pin third-party actions in prPreviewIos.yml The prototype used floating tags (actions/checkout@v5, etc.) which matches the rest of this repo's workflows but is weaker than the extension's prPreview.yml pattern of pinning to commit SHAs. Float tags are a supply-chain risk: if the tag is moved (compromised maintainer, accidental force-push, GitHub incident), the very next run silently executes the new code before any project code does — with the workflow's `contents: write` + `pull-requests: write` token in scope. Pinning to SHAs with version-comment annotations: actions/checkout → 93cb6efe... # v5 actions/setup-node → 48b55a01... # v6 ruby/setup-ruby → 6aaa311d... # v1 (branch, not tag) actions/cache → 00578521... # v4 ruby/setup-ruby is special: their stable pointer is a `v1` BRANCH, not a tag. We pin to whatever commit the branch currently points at; revisit when the branch advances (e.g., on dependabot bumps). Inconsistency note: the rest of the freighter-mobile workflows (ios.yml, ios-e2e.yml, android.yml, etc.) still use floating tags. That's a separate hardening pass worth doing org-wide; not in this PR. Identified by a Nemesis-style multi-pass security audit (stellar/internal-agents nemesis-auditor methodology) as F-001. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: allowlist scripts/gh-ios-env's $GITHUB_ENV writes scripts/gh-ios-env is PR-controlled Ruby code that emits key=value pairs to stdout, which we pipe into $GITHUB_ENV. Without filtering, a modified script could inject arbitrary env into the runner — most dangerously, PATH or LD_PRELOAD overrides that would let subsequent steps' shell resolution find attacker-planted binaries (gh, xcodebuild, etc.) that exfil GH_TOKEN and other env before delegating to the real binary. The allowlist restricts $GITHUB_ENV writes from this script to the seven keys the legitimate script is expected to emit: IOS_SCHEME, FASTLANE_LANE, APP_ID, APP_VERSION, APP_NAME, BUILD_VERSION, ENVFILE NOTE: this is NOT a primary security control. A write-access attacker submitting a malicious PR can modify BOTH this filter and the script in the same diff (pull_request workflows run from PR HEAD's YAML, so hardening in the YAML is itself in the attacker's scope). The real defenses are CODEOWNERS on .github/**, code-review discipline on CI changes, and limiting write access on the repo. This filter is: - a tripwire (removing it from a PR is itself suspicious) - a narrow-scenario defense (catches attackers who modify only the script, not the workflow) - documentation of the script's expected output contract Identified by an adversarial-perspective review of the workflow (Attack 1 chain: PATH poisoning via $GITHUB_ENV). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: apply PR Preview iOS security audit fixes Findings from multi-agent adversarial review of #868: - persist-credentials:false on actions/checkout. Default leaves a GITHUB_TOKEN auth header in .git/config for the rest of the job; any subsequent code execution (yarn lifecycle scripts, build scripts, ./scripts/gh-ios-env) could `git push` via those persisted creds. `gh` uses GH_TOKEN env separately so this doesn't affect the release flow. - Per-PR Cocoapods cache key. Pre-fix key was `${{ runner.os }}-pods-${{ hashFiles('ios/Podfile.lock') }}` with a bare `${{ runner.os }}-pods-` restore-keys prefix. The unscoped key + prefix fallback let any branch's poisoned Pods cache restore into any other branch's build. Scoping the key and restore-prefix with `pr-${{ github.event.pull_request.number }}` confines cache contamination to a single PR. Cost: one cache miss on the first build of each new PR; subsequent pushes to the same PR hit cache. - Replace `gh release delete ... || true` with check-then-delete in both the pre-create step and the cleanup job. `|| true` swallows transient API errors (network, 422 tag-conflict) alongside the not-found case, letting a stale tag survive into the next `gh release create` call (which would silently reuse the existing tag, pointing it at an old commit). - Drop `--target ${PR_HEAD_SHA}` from `gh release create`. With the flag, any account that can click "Publish release" on a preview draft mints an official-looking tag pointing at unreviewed PR-head code — branch protection rules do not extend to release publication. Without the flag, a published draft falls back to main HEAD (reviewed code). The PR's HEAD commit is still recorded in the release notes for traceability. Verifications that came back clean (no code change): all action SHA pins (checkout v5, setup-node v6, ruby/setup-ruby v1, cache v4) match the upstream tag SHAs claimed in comments. ios/Podfile and Gemfile have no `:git`/`:branch` unpinned sources; Podfile.lock EXTERNAL SOURCES section contains only RN-autolinked pods. Residual risk accepted: yarn install + yarn postinstall run with WALLET_KIT_PROJECT_ID_DEV and GITHUB_TOKEN. The secret set is deliberately low-value (dev WalletConnect key, telemetry stubbed, no Apple Connect / Match / Sentry) and any attacker with repo write access has parallel paths to the same outcome; closing this class requires per-PR-push approval friction not warranted at the current secret level. Re-evaluate if Apple Connect / Sentry / signing tokens ever enter the secret set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): handle draft releases in pre-create + cleanup delete steps The check-then-delete logic added in the prior commit (replacing `|| true`) exposed a pre-existing bug masked by the old code: `gh release delete --cleanup-tag` fails with HTTP 422 "Reference does not exist (refs/tags/pr-preview-N)" when the release is a draft, because drafts don't create the git tag until publish. The previous `|| true` silently swallowed this 422 every run (along with real errors we now correctly surface). The fix: query `isDraft` and branch on it — drop `--cleanup-tag` for drafts, keep it for the (defensive) published-release case so a manually- published draft's tag still gets cleaned up by the cleanup job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Enables the previously-drafted
prPreview.ymlworkflow (moves it from.github/workflow-drafts/to.github/workflows/) and applies the Phase 1 security fixes from the multi-agent security review.This is the extension half of the Fullstack PR Preview Phase 1 work. The mobile half (iOS Simulator) is opened as a parallel PR on
stellar/freighter-mobile.What this does
On every PR opened/synchronized/reopened by an SDF org member, the workflow:
keyfield, then re-asserts on the built manifest pre-zip (regression guard against shared Chromium extension IDs with Web Store Freighter — catches both hand-edited keys and build-pipeline injection)gecko.idto a preview-specific UUID to prevent AMO Freighter storage collisionOn PR close, the cleanup job deletes the draft.
Security model
See
Fullstack PR Preview Flow.md§ Security for the full threat model. Highlights:pull_requestonly — neverpull_request_target/issue_comment/workflow_run(the class of vulnerability that hit this org's mobile e2e workflows in Q2 2026). Forbidden-triggers invariant is documented at the top of the YAML.if:gate (head.repo.full_name == github.repository) skips fork PRs cleanly. Real defense is platform-level: fork PRs run with empty secrets and a read-onlyGITHUB_TOKENregardless of YAML. Verified empirically via fork PR test ([TEST — DO NOT REVIEW/MERGE] verify fork-PR YAML provenance #2760, since closed). The author_association check (originally part of the gate) was removed because the field in the webhook event payload only reflects PUBLIC org membership, locking out SDF members with private memberships. Non-SDF gating is now handled by the org-level "Require approval for outside collaborators" setting plus platform-level fork-PR secret-withholding.permissions: {};contents: writegranted per-job only where needed.actions/checkoutruns withpersist-credentials: falseso the GITHUB_TOKEN auth header is not left in.git/configfor subsequent steps to abuse viagit push.gh release deleteis check-then-delete (not|| true), so a cancelled cleanup leaves no permanent half-state and transient API errors fail loudly instead of being masked.gh release createdoes NOT pass--target. If a draft is ever manually published, the tag falls back to master HEAD (reviewed code) rather than the PR's HEAD commit. Closes a one-click escalation path where any account that can click "Publish release" on a draft would otherwise mint an official-looking release containing arbitrary PR-head code (branch protection rules do not extend to release publication). The PR's HEAD commit is still recorded in the release notes for traceability.Draft-release visibility — already tested
Confirmed during design (stellar/freighter test draft, 2026-05-08):
Third-party action surface
Replaced three third-party actions with inline
jq/zipshell:jossef/action-set-json-field@2a0f7d95(×2)restackio/update-json-file-action@f8ef1561montudor/action-zip@0852c269FROM alpine:latest(SHA pin doesn't freeze the transitive image)actions/checkout@de0fac2e(v6) andactions/setup-node@53b83947(v6.3.0) kept — official GitHub-maintained actions. SHA pins verified against upstream release tag SHAs.Test plan
pr-preview-Nis created on stellar/freighter, sticky comment is posted, install instructions render correctly"key": "..."to the source manifest fails CI (manifest-key assertion regression guard)"key"into the built manifest post-build also fails CI (post-build re-assertion)Related
Fullstack PR Preview Flow(Phase 1 rollout)pr-preview-workflow-security-review.md🤖 Generated with Claude Code