Skip to content

chore(shorebird): post-merge hardening follow-ups from #7202 #7774

Closed

Description

@rabble

Post-merge follow-ups from the review of #7202 (merged as 2504f4d871). The review landed after the merge, so these were never actioned. None is a defect in what shipped; all are hardening or documentation gaps in the new Shorebird release/patch path.

These findings are from an automated pass and have not been human-verified.

1. RELEASE_COMMIT is unverified against RELEASE_VERSION, and fails open

The patch-safety story — print the diff since the release, block native/asset/dependency changes — rests entirely on the operator typing the right commit into the ios-patch / android-patch input.

  • A too-old commit is safe: it over-blocks.
  • A too-new commit (e.g. current main) makes BLOCKED_PATCH_PATHS empty, so every gate in prepare_patch_source passes vacuously while the patch still carries everything since the real release.

Shorebird's own native/asset refusal is still the backstop, so this isn't catastrophic — but a gate advertised as a gate silently becomes a no-op, and it fails open rather than closed.

Cheap partial fix: cross-check the build name in git show $RELEASE_COMMIT:pubspec.yaml against the 1.0.20 half of RELEASE_VERSION. Better: have build_aab / build_ios push a tag at the release commit so the patch workflow resolves it instead of trusting input.

2. Patch dart-defines come from current env-group values, not the release's

mobile/docs/SHOREBIRD_CODE_PUSH.md warns that DEFAULT_ENV is "the easiest thing to get wrong". The same silent-drift risk applies to every other key in dart_defines.json, and those have no operator input at all — nobody gets a chance to notice.

FF_DIVINE_SUPPORTERS is read via bool.fromEnvironment in lib/features/feature_flags/services/build_configuration.dart:52, so flipping it in the Codemagic group means the next "Dart-only bug fix" patch also ships a feature flip to production. Same shape for PROOFMODE_SIGNING_SERVER_ENDPOINT / _TOKEN and SUPPORTERS_API_BASE_URL.

Suggested: widen the doc warning past DEFAULT_ENV, and record a SHA-256 of the release's dart_defines.json so a patch run can print a mismatch.

3. Zapstore APK and Play AAB are now built on different engines, undocumented

build_apk is stock Flutter 3.44.9; build_aab is Shorebird's 3.44.9 fork. #7202's own justification for the pin — the 3.44.0 Shorebird engine crashing in the cupertino_http async FFI path — is proof the delta is not theoretical. A Shorebird-engine-specific crash reproduces on Play and not on Zapstore, and vice versa.

The doc table records that the APKs aren't patchable but not that they're a different engine. That's the line a future triage will need.

4. curl … install.sh | bash from a mutable main now sits on the store-signing path

Before #7202, Shorebird wasn't installed in ios-build / android-build at all. EXPECTED_SHOREBIRD_VERSION validates identity, not integrity — a tampered installer reports whatever version it likes. Pinning the URL to a commit SHA instead of main is cheap.

Related: $HOME/.shorebird is now cached into both release workflows, so a poisoned cache would also pass the version check. Dropping the cache entirely costs a multi-GB engine re-download, so that's a real tradeoff — but caching only ~/.shorebird/bin/cache/flutter while reinstalling the CLI binary each run gets most of the speed with less trust surface.

Nits

  • .github/scripts/tests/test_codemagic_android_build_number.py now covers Flutter pin parity, Shorebird gating, YAML parsing and runbook prose. The filename is actively misleading; rename it.
  • ci-config-tests goes into the results array in mobile_ci.yaml rather than beside the explicit always-required codemagic-groups check. It works — a failure is caught in both the app-ci-required true and false branches — but it's inconsistent with the comment that exists specifically to explain why ungated always-required guards get their own check.
  • Pre-existing but sharper now: build_apk uses $PROJECT_BUILD_NUMBER while build_aab uses the Play-floor number, so the GitHub-release APK and the Shorebird release version from one run can differ. One line in the doc table would prevent a triage mistake.
  • mobile/packages/caption_generator/android/build.gradle.kts: flutterRoot throws a clear GradleException when unset, but flutterDebugEmbeddingJar has no existence check — a missing jar degrades to an unresolved-reference compile error rather than a named failure.

Not in scope here

The DPA / store data-sharing question from #7202's description is an owner decision with a clock on it — the next store build is the Shorebird cutover — and is tracked separately rather than in this cleanup issue.

Activity

  1. NotThatKindOfDrLiz commented on Aug 18, 2026

    @NotThatKindOfDrLiz
    Member

    Closing as superseded by #7776. All overlapping hardening requirements are already tracked there, and the remaining engine-delta documentation, test rename, CI guard placement, APK/AAB build-number documentation, and caption-generator JAR validation items have been transferred into #7776's consolidated checklist.

  2. moved this from Triage to Testing in Divine programon Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions