SELF-2469: SDK Distribution: Hosted URL loading + native shell publishing#1938
SELF-2469: SDK Distribution: Hosted URL loading + native shell publishing#1938jcortejoso wants to merge 19 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CI workflows for publishing Android SDK and deploying WebView; introduces Android/iOS SDK packaging and docs; updates WebView app build/hosting (basename, Vite base, Vercel rewrites/headers); and changes default SDK remote base URLs from the alpha host to https://verify.self.xyz/v1/. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
SD-03: Configure webview-app for Vercel deployment at /v1/ base path. - Make Vite base path configurable via VITE_BASE_PATH env var - Add BrowserRouter basename derived from import.meta.env.BASE_URL - Add build:hosted script producing dist/v1/ directory structure - Add vercel.json with SPA rewrites, caching headers, and build config - Add GitHub Actions workflow for Vercel deployment on push to main Default embedded mode (base: './') is unchanged. Hosted mode activates when VITE_BASE_PATH=/v1/ is set at build time. Ref: SELF-2469
Add maven-publish plugin with groupId=xyz.self.sdk, artifactId=native-shell-android, version=0.1.0. Verified: publishToMavenLocal succeeds, AAR contains no bundled web assets, POM includes correct runtime dependencies. Ref: SELF-2473
- Add GitHubPackages maven repository to publishing block (credentials from GITHUB_ACTOR/GITHUB_TOKEN env or gpr.user/gpr.token properties) - Add publish-android-sdk.yml workflow (manual dispatch with version input) - Builds, tests, then publishes release AAR to GitHub Packages Ref: SELF-2473
Defaults to dry-run (publishToMavenLocal) so the workflow can be tested safely. Uncheck dry-run to publish to GitHub Packages. Ref: SELF-2473
SPM, CocoaPods (via Git), and GitHub Packages (Maven) instructions with credential setup and requirements. Ref: SELF-2474
…self.xyz Update default URL in SelfVerificationActivity and test assertions to match the new hosted URL after rebase onto dev.
2b20b0a to
7e80e65
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/native-shell-android/src/main/kotlin/xyz/self/sdk/webview/AndroidWebViewHost.kt (1)
136-141:⚠️ Potential issue | 🔴 Critical
remoteWebAppBaseUrlis no longer an exclusive allowlist.
shouldAllowNavigation()andonPermissionRequest()always trust the default hosted origin even when callers pass a differentremoteWebAppBaseUrlornull. A redirect orwindow.locationchange can therefore escape a custom-host or bundled-only configuration and keep the bridge plus camera/mic access on an origin the caller did not authorize.Based on learnings, "Fail closed on security-critical boundaries. Default-deny for protocol compatibility, remote bundle loading, and verification session lifecycle. Reject unknown protocol versions, block remote devServerUrl in production".
Also applies to: 246-253, 346-347
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5fb09f5f-0e30-4e04-9398-4844831a9d4a
📒 Files selected for processing (22)
.github/workflows/publish-android-sdk.yml.github/workflows/webview-deploy.ymlpackages/native-shell-android/README.mdpackages/native-shell-android/build.gradle.ktspackages/native-shell-android/src/main/kotlin/xyz/self/sdk/api/SelfSdkConfig.ktpackages/native-shell-android/src/main/kotlin/xyz/self/sdk/webview/AndroidWebViewHost.ktpackages/native-shell-android/src/test/kotlin/xyz/self/sdk/api/SelfSdkConfigTest.ktpackages/native-shell-android/src/test/kotlin/xyz/self/sdk/api/SelfSdkLaunchTest.ktpackages/native-shell-android/src/test/kotlin/xyz/self/sdk/webview/AndroidWebViewHostTest.ktpackages/native-shell-ios/.gitignorepackages/native-shell-ios/Package.swiftpackages/native-shell-ios/README.mdpackages/native-shell-ios/SelfSDK.podspecpackages/native-shell-ios/Sources/SelfNativeShell/API/SelfSdkConfig.swiftpackages/native-shell-ios/Sources/SelfNativeShell/WebView/SelfWebViewHost.swiftpackages/native-shell-ios/Tests/SelfNativeShellTests/RemoteNavigationPolicyTests.swiftpackages/native-shell-ios/Tests/SelfNativeShellTests/SelfSdkConfigTests.swiftpackages/webview-app/package.jsonpackages/webview-app/scripts/prepare-hosted.mjspackages/webview-app/src/App.tsxpackages/webview-app/vercel.jsonpackages/webview-app/vite.config.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/native-shell-android/README.md (1)
16-17:⚠️ Potential issue | 🟠 Major
settings.gradle.ktssample uses an unavailable API and will break consumer setup.Line 16 and Line 17 use
project.findProperty(...), which is not valid in asettings.gradle.ktscontext. Consumers following this README will fail dependency configuration.Suggested fix
dependencyResolutionManagement { repositories { google() mavenCentral() maven { url = uri("https://maven.pkg.github.com/selfxyz/self") credentials { - username = project.findProperty("gpr.user") as String? - password = project.findProperty("gpr.token") as String? + username = providers.gradleProperty("gpr.user").orNull + password = providers.gradleProperty("gpr.token").orNull } } } }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ca82f66-d406-4a20-8396-4058a373c9f4
📒 Files selected for processing (17)
.github/workflows/publish-android-sdk.yml.github/workflows/webview-deploy.ymlpackages/native-shell-android/README.mdpackages/native-shell-android/build.gradle.ktspackages/native-shell-android/src/main/kotlin/xyz/self/sdk/api/SelfSdkConfig.ktpackages/native-shell-android/src/main/kotlin/xyz/self/sdk/webview/AndroidWebViewHost.ktpackages/native-shell-android/src/main/kotlin/xyz/self/sdk/webview/SelfVerificationActivity.ktpackages/native-shell-android/src/test/kotlin/xyz/self/sdk/webview/AndroidWebViewHostSecurityTest.ktpackages/native-shell-ios/.gitignorepackages/native-shell-ios/README.mdpackages/native-shell-ios/SelfSDK.podspecpackages/native-shell-ios/Sources/SelfNativeShell/API/SelfSdkConfig.swiftpackages/webview-app/package.jsonpackages/webview-app/scripts/prepare-hosted.mjspackages/webview-app/src/App.tsxpackages/webview-app/vercel.jsonpackages/webview-app/vite.config.ts
✅ Files skipped from review due to trivial changes (9)
- packages/native-shell-ios/.gitignore
- packages/webview-app/package.json
- packages/webview-app/scripts/prepare-hosted.mjs
- packages/native-shell-android/src/main/kotlin/xyz/self/sdk/webview/SelfVerificationActivity.kt
- packages/native-shell-android/src/main/kotlin/xyz/self/sdk/api/SelfSdkConfig.kt
- packages/native-shell-ios/SelfSDK.podspec
- .github/workflows/webview-deploy.yml
- packages/native-shell-android/src/main/kotlin/xyz/self/sdk/webview/AndroidWebViewHost.kt
- packages/native-shell-android/build.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/webview-app/vite.config.ts
- packages/webview-app/vercel.json
- packages/native-shell-ios/Sources/SelfNativeShell/API/SelfSdkConfig.swift
|
@codex review |
- README: use providers.gradleProperty() instead of project.findProperty() (settings.gradle.kts uses Settings, not Project) - package.json: restore tsc --noEmit in build:hosted to catch TS errors - vercel.json: add /v1/ cache header rules for hosted deployment paths - publish workflow: add version input validation to prevent shell injection Ref: SELF-2469
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e80e65bb3
ℹ️ 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".
Agent-Logs-Url: https://github.com/selfxyz/self/sessions/9d84d455-946f-436a-882e-053222a6c9f8 Co-authored-by: jcortejoso <5635989+jcortejoso@users.noreply.github.com>
|
Reviewed all changes. Found one stale reference missed during the SD-02 migration: Other observations (no code changes needed):
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 93f1966d-bf61-48e9-8516-5663bbb60fc8
📒 Files selected for processing (6)
.github/workflows/publish-android-sdk.ymlpackages/native-shell-android/README.mdpackages/native-shell-ios/Sources/SelfNativeShell/WebView/SelfWebViewHost.swiftpackages/native-shell-ios/Tests/SelfNativeShellTests/SelfWebViewHostTests.swiftpackages/webview-app/package.jsonpackages/webview-app/vercel.json
✅ Files skipped from review due to trivial changes (3)
- packages/webview-app/package.json
- packages/webview-app/vercel.json
- packages/native-shell-android/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/publish-android-sdk.yml
- vercel.json: add /v1/:path* rewrite to /v1/index.html for hosted deployment (native shells load from verify.self.xyz/v1/) - webview-deploy.yml: add common/** to path triggers since @selfxyz/common is an explicit build dependency Ref: SELF-2469
- Fix last self-app-alpha.vercel.app reference in SelfWebViewHostTests - Add --prebuilt to vercel-args so the workflow uses our build:hosted output instead of Vercel re-running its own buildCommand Ref: SELF-2469
Align WebViewProviderImpl default remote URL and test assertions with the production hosted URL. Avoids split production behavior between native-shell-ios and self-sdk-swift. Ref: SELF-2469
|
@copilot review |
…ity tests Agent-Logs-Url: https://github.com/selfxyz/self/sessions/0d67dd0f-cd2d-4aaa-9143-4e25652d681e Co-authored-by: jcortejoso <5635989+jcortejoso@users.noreply.github.com>
|
Reviewed all 21 files in the PR. Found one remaining issue: three Android security tests ( The Everything else looks correct:
|
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3f6c71edf
ℹ️ 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".
- actions/checkout v4.3.1 (34e11487) - actions/setup-java v4.8.0 (c1e32368) - amondnet/vercel-action v25.2.0 (16e87c0a) Ref: SELF-2469
Summary
Three URL constants and test fixtures still referenced
https://self-app-alpha.vercel.appafter the SD-01/SD-02 migration tohttps://verify.self.xyz/v1/:SelfWebViewHost.defaultRemoteBaseURL(iOS) — internal fallback used when no URL is passed to the host directly;SelfSdkConfigwas already updated but this constant was missedSelfWebViewHostTests(iOS) — three tests asserting against the stale default (testReleaseBuildUsesRemoteOrigin,testAllowedNavigationAcceptsRemoteAlphaOrigin,testBridgeTrustAcceptsRemoteRejectsDidit)AndroidWebViewHostSecurityTest— three tests usingself-app-alpha.vercel.appas explicitremoteBasefor navigation-policy behavior tests (navigation allows remote didit and debug origins,didit on non-443 port is rejected,bridge trust accepts remote rejects didit and arbitrary origins)Note:
packages/kmp-sdkstill contains stale references — out of scope for this PR (separate package).Test plan
SelfWebViewHostTestssuite updated and passingAndroidWebViewHostSecurityTestsuite updated and passingNative Consolidation Checklist
cd app && yarn jest:run/yarn workspace @selfxyz/rn-sdk-test-app test)