Publish channel-aware update metadata for Lantern releases#8903
Open
atavism wants to merge 1 commit into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
|
|
||
| def parse_appcast(xml_text: str) -> tuple[str, list[dict[str, str]]]: | ||
| root = ET.fromstring(xml_text) |
📝 WalkthroughWalkthroughThis PR introduces a beta-aware update pipeline: new CI scripts generate per-artifact update-metadata sidecars and verify the update service against beta/stable channels, both wired into release.yml and a new reusable verify-update-service.yml workflow. Client-side AppUrls, macOS appcast feed URL, and Android sideload defaults are updated for beta channel support. ChangesBeta Update Service Rollout
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow as release.yml
participant Generator as generate_update_metadata.py
participant GitHubRelease as GitHub Release/S3
participant VerifyWorkflow as verify-update-service.yml
participant Verifier as verify_update_service.py
participant UpdateService as Update Service
ReleaseWorkflow->>Generator: run generator on downloaded installer artifacts
Generator->>Generator: classify artifact, compute sha256, sign via dart
Generator-->>ReleaseWorkflow: *.update.json sidecars
ReleaseWorkflow->>GitHubRelease: upload sidecars to S3 and draft release
ReleaseWorkflow->>VerifyWorkflow: trigger verify-update-service (beta channel/version)
VerifyWorkflow->>Verifier: run verify_update_service.py
loop poll until timeout
Verifier->>UpdateService: POST/GET beta and stable endpoints
UpdateService-->>Verifier: version, checksum, appcast XML
Verifier->>Verifier: validate version, signature, URL suffix
end
Verifier-->>VerifyWorkflow: success or SystemExit
VerifyWorkflow-->>ReleaseWorkflow: verify-update-service result
ReleaseWorkflow->>ReleaseWorkflow: gate release-finalize on publish-update-metadata success
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| import subprocess | ||
| import sys | ||
| import tempfile | ||
| import unittest |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Lantern-side support for channel-aware auto-updates by pointing desktop clients at the lantern-cloud update service, generating per-artifact update metadata during release CI, and verifying the public update service after beta releases publish.
Changes:
- Switch macOS Sparkle feed URL and in-app desktop updater feed selection to
https://update.getlantern.org/update/lantern/...with channel query params. - Add CI tooling to generate/update per-artifact update metadata sidecars and to verify the update service post-release (with unit tests).
- Update release workflow to publish metadata sidecars and run beta-only post-publish update verification.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/core/common/app_urls_test.dart | Adds coverage for build-type → appcast URL mapping. |
| scripts/ci/verify_update_service.py | New script to poll/validate beta updates and ensure stable doesn’t leak beta. |
| scripts/ci/verify_update_service_test.py | Unit tests for the update-service verifier script. |
| scripts/ci/generate_update_metadata.py | New script to generate update-metadata sidecars for release artifacts. |
| scripts/ci/generate_update_metadata_test.py | Unit tests for the metadata sidecar generator. |
| macos/Runner/Info.plist | Updates Sparkle SUFeedURL to the channel-aware update service endpoint. |
| lib/core/updater/android_sideload_updater.dart | Enables Android sideload auto-updates by default on beta builds. |
| lib/core/common/app_urls.dart | Centralizes update-service base URL and updates appcast endpoints. |
| .github/workflows/verify-update-service.yml | Adds a reusable workflow to run verifier tests and perform polling verification. |
| .github/workflows/release.yml | Publishes update metadata sidecars during release and runs beta post-publish verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if filename.endswith(".exe"): | ||
| return "windows", "windows", "amd64" | ||
| if filename.endswith(".apk"): | ||
| return "android", "android", "arm" |
Comment on lines
+43
to
+47
| "lantern-installer-beta.dmg": ("macos", "darwin", "amd64"), | ||
| "lantern-installer-beta.exe": ("windows", "windows", "amd64"), | ||
| "lantern-installer-beta.apk": ("android", "android", "arm"), | ||
| "lantern-installer-beta.deb": ("linux", "linux", "amd64"), | ||
| "lantern-installer-beta-arm64.deb": ("linux", "linux", "arm64"), |
Comment on lines
+81
to
+85
| "platform": "android", | ||
| "os": "android", | ||
| "arch": "arm", | ||
| "filename": "lantern-installer-beta.apk", | ||
| "url": ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/core/common/app_urls.dart (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider making
androidSideloadUpdateEndpointaconst.
updateServiceLanternisstatic constbutandroidSideloadUpdateEndpointisstatic Stringdespite being assigned a compile-time constant. Making itconstwould be consistent with the other URL constants.♻️ Proposed fix
- static String androidSideloadUpdateEndpoint = updateServiceLantern; + static const androidSideloadUpdateEndpoint = updateServiceLantern;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/core/common/app_urls.dart` at line 24, Make androidSideloadUpdateEndpoint in AppUrls a compile-time constant to match updateServiceLantern and the other URL constants. Update the static field declaration so it is const rather than a mutable String, keeping the assignment to updateServiceLantern unchanged..github/workflows/release.yml (1)
978-984: 🧹 Nitpick | 🔵 Trivial
verify-update-serviceruns post-publish, so a failed check cannot block the release.This job gates on
release-finalize.result == 'success', andrelease-finalizehas already flipped the draft to public (gh release edit --draft=false). If the update-service check fails, the beta release is already live; the failure only surfaces as a red job with no rollback. If the intent is to catch a broken beta update feed before users see it, consider verifying before finalize (or documenting this as an intentionally informational post-publish signal).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 978 - 984, The `verify-update-service` job in the release workflow is running after `release-finalize`, so it cannot prevent a bad beta release from going live. Move the update-service verification logic to run before `release-finalize` (using the same `verify-update-service`/`release-finalize` gating symbols) if it is meant to block publication, or otherwise document that this job is only an informational post-publish check.scripts/ci/generate_update_metadata_test.py (1)
60-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test case for a non-beta build type to cover URL/channel normalization.
test_sidecar_for_builds_json_shapeonly testsbuild_type="beta", where the rawbuild_typeand normalizedchannelhappen to match. A test using"production"(→ channel"stable") would verify the URL path and catch the divergence flagged ingenerate_update_metadata.py:74-75.🧪 Suggested additional test
+ def test_sidecar_for_normalizes_channel_in_url(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + artifact = pathlib.Path(tmp) / "lantern-installer-production.deb" + artifact.write_bytes(b"deb bytes") + + metadata = generate_update_metadata.sidecar_for( + artifact, + "production", + "9.2.0", + "lantern.io", + ) + + self.assertEqual(metadata["channel"], "stable") + self.assertEqual(metadata["build_type"], "production") + self.assertIn("/releases/stable/9.2.0/", metadata["url"])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/generate_update_metadata_test.py` around lines 60 - 92, The current sidecar test only covers the beta path where build_type and channel are identical, so it misses URL/channel normalization. Add a new test in test_sidecar_for_builds_json_shape or a sibling test that calls generate_update_metadata.sidecar_for with build_type set to production and asserts the resulting channel is stable while the URL path uses the normalized channel. Ensure the assertions still cover the returned metadata shape and the relevant sidecar_for behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/verify-update-service.yml:
- Around line 80-88: The “Run update service verifier” step is passing GitHub
Actions inputs directly into the shell command, which can allow shell injection
if a value like version contains metacharacters. Move the inputs from the run
block into environment variables on the same step, then update the
verify_update_service.py invocation to read from those env vars instead of
interpolating `${{ inputs.* }}` inside the shell command. Keep the change
localized to the workflow step that calls scripts/ci/verify_update_service.py.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 978-984: The `verify-update-service` job in the release workflow
is running after `release-finalize`, so it cannot prevent a bad beta release
from going live. Move the update-service verification logic to run before
`release-finalize` (using the same `verify-update-service`/`release-finalize`
gating symbols) if it is meant to block publication, or otherwise document that
this job is only an informational post-publish check.
In `@lib/core/common/app_urls.dart`:
- Line 24: Make androidSideloadUpdateEndpoint in AppUrls a compile-time constant
to match updateServiceLantern and the other URL constants. Update the static
field declaration so it is const rather than a mutable String, keeping the
assignment to updateServiceLantern unchanged.
In `@scripts/ci/generate_update_metadata_test.py`:
- Around line 60-92: The current sidecar test only covers the beta path where
build_type and channel are identical, so it misses URL/channel normalization.
Add a new test in test_sidecar_for_builds_json_shape or a sibling test that
calls generate_update_metadata.sidecar_for with build_type set to production and
asserts the resulting channel is stable while the URL path uses the normalized
channel. Ensure the assertions still cover the returned metadata shape and the
relevant sidecar_for behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c3568237-e869-4dbc-9724-504fc2a8e311
📒 Files selected for processing (10)
.github/workflows/release.yml.github/workflows/verify-update-service.ymllib/core/common/app_urls.dartlib/core/updater/android_sideload_updater.dartmacos/Runner/Info.plistscripts/ci/generate_update_metadata.pyscripts/ci/generate_update_metadata_test.pyscripts/ci/verify_update_service.pyscripts/ci/verify_update_service_test.pytest/core/common/app_urls_test.dart
Comment on lines
+80
to
+88
| - name: Run update service verifier | ||
| run: | | ||
| python3 scripts/ci/verify_update_service.py \ | ||
| --update-url "${{ inputs.update_url }}" \ | ||
| --channel "${{ inputs.channel }}" \ | ||
| --platform "${{ inputs.platform }}" \ | ||
| --version "${{ inputs.version }}" \ | ||
| --timeout-seconds "${{ inputs.timeout_seconds }}" \ | ||
| --interval-seconds "${{ inputs.interval_seconds }}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pass inputs via environment variables to prevent shell injection.
${{ inputs.* }} expressions are expanded by GitHub Actions before the shell executes. If any input (especially version, which may come from a git tag) contains shell metacharacters like $(cmd) or ; cmd, arbitrary code would execute on the runner. Exploitability is limited to users with write access, but using env vars is a standard GitHub Actions hardening practice.
🛡️ Proposed fix using environment variables
- name: Run update service verifier
+ env:
+ UPDATE_URL: ${{ inputs.update_url }}
+ CHANNEL: ${{ inputs.channel }}
+ PLATFORM: ${{ inputs.platform }}
+ VERSION: ${{ inputs.version }}
+ TIMEOUT_SECONDS: ${{ inputs.timeout_seconds }}
+ INTERVAL_SECONDS: ${{ inputs.interval_seconds }}
run: |
python3 scripts/ci/verify_update_service.py \
- --update-url "${{ inputs.update_url }}" \
- --channel "${{ inputs.channel }}" \
- --platform "${{ inputs.platform }}" \
- --version "${{ inputs.version }}" \
- --timeout-seconds "${{ inputs.timeout_seconds }}" \
- --interval-seconds "${{ inputs.interval_seconds }}"
+ --update-url "$UPDATE_URL" \
+ --channel "$CHANNEL" \
+ --platform "$PLATFORM" \
+ --version "$VERSION" \
+ --timeout-seconds "$TIMEOUT_SECONDS" \
+ --interval-seconds "$INTERVAL_SECONDS"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Run update service verifier | |
| run: | | |
| python3 scripts/ci/verify_update_service.py \ | |
| --update-url "${{ inputs.update_url }}" \ | |
| --channel "${{ inputs.channel }}" \ | |
| --platform "${{ inputs.platform }}" \ | |
| --version "${{ inputs.version }}" \ | |
| --timeout-seconds "${{ inputs.timeout_seconds }}" \ | |
| --interval-seconds "${{ inputs.interval_seconds }}" | |
| - name: Run update service verifier | |
| env: | |
| UPDATE_URL: ${{ inputs.update_url }} | |
| CHANNEL: ${{ inputs.channel }} | |
| PLATFORM: ${{ inputs.platform }} | |
| VERSION: ${{ inputs.version }} | |
| TIMEOUT_SECONDS: ${{ inputs.timeout_seconds }} | |
| INTERVAL_SECONDS: ${{ inputs.interval_seconds }} | |
| run: | | |
| python3 scripts/ci/verify_update_service.py \ | |
| --update-url "$UPDATE_URL" \ | |
| --channel "$CHANNEL" \ | |
| --platform "$PLATFORM" \ | |
| --version "$VERSION" \ | |
| --timeout-seconds "$TIMEOUT_SECONDS" \ | |
| --interval-seconds "$INTERVAL_SECONDS" |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 83-83: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 84-84: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 85-85: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 86-86: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/verify-update-service.yml around lines 80 - 88, The “Run
update service verifier” step is passing GitHub Actions inputs directly into the
shell command, which can allow shell injection if a value like version contains
metacharacters. Move the inputs from the run block into environment variables on
the same step, then update the verify_update_service.py invocation to read from
those env vars instead of interpolating `${{ inputs.* }}` inside the shell
command. Keep the change localized to the workflow step that calls
scripts/ci/verify_update_service.py.
Source: Linters/SAST tools
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.
Resolves https://github.com/getlantern/engineering/issues/3661
Adds the Lantern side of channel-aware auto-updates: desktop builds now point at the lantern-cloud appcast endpoint, release CI publishes update metadata for artifacts, and beta releases verify the public update service after publishing.
Summary by CodeRabbit
New Features
Bug Fixes