diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 4cc53a1e..618b4859 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -15,7 +15,7 @@ jobs: check_changes: runs-on: ubuntu-latest # npm --provenance requires a GitHub-hosted runner # Room for the native-required wait below. - timeout-minutes: 60 + timeout-minutes: 130 outputs: harmony_changed: ${{ steps.check.outputs.changed }} prev_tag: ${{ steps.check.outputs.prev_tag }} @@ -31,17 +31,24 @@ jobs: GH_TOKEN: ${{ github.token }} SHA: ${{ github.sha }} run: | - for attempt in $(seq 1 45); do + # The gate turns green only after the slowest native E2E: e2e-ios + # is a build job (timeout 45 min) followed by its test legs + # (timeout 60 min), plus macOS queueing. A green release commit + # took 55 min (10.60.0), so 45 failed releases cut right after the + # push; wait out both budgets instead. + # Checks at minute 0 through 120: the last poll lands on the + # boundary itself, with no trailing sleep before giving up. + for attempt in $(seq 0 120); do state="$(gh api "repos/$GITHUB_REPOSITORY/commits/$SHA/status" \ --jq '[.statuses[] | select(.context == "native-required")][0].state // "missing"')" echo "native-required for $SHA: $state" case "$state" in success) exit 0 ;; - pending|missing) sleep 60 ;; + pending|missing) if [ "$attempt" -lt 120 ]; then sleep 60; fi ;; *) echo "::error::native-required is $state for $SHA; fix CI before publishing"; exit 1 ;; esac done - echo "::error::native-required for $SHA did not turn green within 45 minutes" + echo "::error::native-required for $SHA did not turn green within 120 minutes" exit 1 - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/android/src/main/java/cn/reactnative/modules/update/UpdateContext.java b/android/src/main/java/cn/reactnative/modules/update/UpdateContext.java index e6b2812c..c8eec65e 100644 --- a/android/src/main/java/cn/reactnative/modules/update/UpdateContext.java +++ b/android/src/main/java/cn/reactnative/modules/update/UpdateContext.java @@ -63,7 +63,7 @@ public class UpdateContext { // 4 (clear first time) exists in the state core but has no Android caller. private static final int STATE_OP_CLEAR_ROLLBACK_MARK = 5; private static final int STATE_OP_RESOLVE_LAUNCH = 6; - private static final String KEY_FIRST_LOAD_MARKED = "firstLoadMarked"; + static final String KEY_FIRST_LOAD_MARKED = "firstLoadMarked"; // Bumped by resetToPackagedBundle. The cold-start check runs for minutes // and may already hold a decision when the app resets to the packaged // bundle; the orchestrator samples this counter and abandons activation @@ -134,7 +134,7 @@ private UpdateContext(Context context) { // Execute cleanUp before clearing SharedPreferences to avoid race condition cleanUp(DownloadTaskParams.DEFAULT_MAX_AGE_DAYS); SharedPreferences.Editor editor = this.sp.edit(); - editor.clear(); + removeVersionScopedKeys(editor); applyState(editor, nextState); persistEditor(editor, "sync state with binary version"); clearNativeCheckCache(); @@ -436,6 +436,15 @@ private static void putNullableString( } } + // Not editor.clear(): that also wiped the uuid (re-bucketing every gray + // release on each app upgrade) and the native config (no crash hold and + // no cold-start round until JS ran again). See VersionScopedKeys. + private void removeVersionScopedKeys(SharedPreferences.Editor editor) { + for (String key : VersionScopedKeys.select(sp.getAll().keySet())) { + editor.remove(key); + } + } + private void applyState(SharedPreferences.Editor editor, StateCoreResult state) { putNullableString(editor, "packageVersion", state.packageVersion); putNullableString(editor, "buildTime", state.buildTime); @@ -618,8 +627,10 @@ public void markSuccess() { * (so the next launch resolves to the built-in bundle) and delete the * downloaded versions, keeping only the directory of the version this * process is running from (a silent reset must not break its on-demand - * asset loads). Only the client uuid survives — it identifies the install - * for gray release bucketing and must not change on reset. + * asset loads). The install's own keys survive (VersionScopedKeys): the + * client uuid identifies the install for gray release bucketing and must + * not change on reset, and the native config stays with the host, as on + * iOS and Harmony. */ public void resetToPackagedBundle() { synchronized (commitLock) { @@ -637,18 +648,14 @@ private void resetToPackagedBundleLocked() { resetState.buildTime = getBuildTime(); resetState.firstTime = false; resetState.firstTimeOk = true; - String uuid = sp.getString("uuid", null); // The cached check response still advertises the version this reset // removes; drop it first so a failure aborts before the state wipe. if (!clearNativeCheckCache()) { throw new IllegalStateException("Failed to persist state for reset check cache"); } SharedPreferences.Editor editor = sp.edit(); - editor.clear(); + removeVersionScopedKeys(editor); applyState(editor, resetState); - if (uuid != null) { - editor.putString("uuid", uuid); - } persistEditorOrThrow(editor, "reset to packaged bundle"); ignoreRollback = false; Log.i(TAG, "Reset to packaged bundle"); diff --git a/android/src/main/java/cn/reactnative/modules/update/VersionScopedKeys.java b/android/src/main/java/cn/reactnative/modules/update/VersionScopedKeys.java new file mode 100644 index 00000000..325be94b --- /dev/null +++ b/android/src/main/java/cn/reactnative/modules/update/VersionScopedKeys.java @@ -0,0 +1,39 @@ +package cn.reactnative.modules.update; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +/** + * The keys of the main state file that describe installed versions rather + * than the install itself: per-version info (hash_*), the first-load marker, + * the native round's incomplete breadcrumb and the response cache's legacy + * location. A binary upgrade or a reset to the packaged bundle drops exactly + * these (plus the state fields applyState rewrites). Everything else — the + * client uuid that buckets gray releases, the persisted native config the + * cold-start round and crash hold run from, the bundle hash cache keyed by + * package version — belongs to the install and survives, as on iOS and + * Harmony. + */ +final class VersionScopedKeys { + static final String HASH_INFO_PREFIX = "hash_"; + + private VersionScopedKeys() { + } + + static List select(Collection keys) { + List doomed = new ArrayList<>(); + for (String key : keys) { + if (key == null) { + continue; + } + if (key.startsWith(HASH_INFO_PREFIX) + || key.equals(UpdateContext.KEY_FIRST_LOAD_MARKED) + || key.equals(SyncCoordinator.KEY_ROUND_INCOMPLETE) + || key.equals(SyncCoordinator.KEY_RESP_CACHE)) { + doomed.add(key); + } + } + return doomed; + } +} diff --git a/android/src/test/java/cn/reactnative/modules/update/VersionScopedKeysTest.java b/android/src/test/java/cn/reactnative/modules/update/VersionScopedKeysTest.java new file mode 100644 index 00000000..64b5491d --- /dev/null +++ b/android/src/test/java/cn/reactnative/modules/update/VersionScopedKeysTest.java @@ -0,0 +1,43 @@ +package cn.reactnative.modules.update; + +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import org.junit.Test; + +public class VersionScopedKeysTest { + @Test + public void dropsVersionKeysAndKeepsInstallKeys() { + List doomed = VersionScopedKeys.select(Arrays.asList( + "uuid", + "nativeConfig", + "bundleHashCache", + "packageVersion", + "currentVersion", + "hash_abc", + "hash_def", + "firstLoadMarked", + "nativeCheckIncomplete", + "nativeCheckResp", + null + )); + assertEquals( + new HashSet<>(Arrays.asList( + "hash_abc", + "hash_def", + "firstLoadMarked", + "nativeCheckIncomplete", + "nativeCheckResp")), + new HashSet<>(doomed)); + } + + @Test + public void keyNamesMatchTheirOwners() { + assertEquals("firstLoadMarked", UpdateContext.KEY_FIRST_LOAD_MARKED); + assertEquals("nativeCheckIncomplete", SyncCoordinator.KEY_ROUND_INCOMPLETE); + assertEquals("nativeCheckResp", SyncCoordinator.KEY_RESP_CACHE); + assertEquals("nativeConfig", SyncCoordinator.KEY_CONFIG); + } +} diff --git a/harmony/pushy/src/main/ets/ArchiveLimits.ts b/harmony/pushy/src/main/ets/ArchiveLimits.ts index f3a2a259..9591a935 100644 --- a/harmony/pushy/src/main/ets/ArchiveLimits.ts +++ b/harmony/pushy/src/main/ets/ArchiveLimits.ts @@ -1,7 +1,6 @@ import fileIo from '@ohos.file.fs'; import statvfs from '@ohos.file.statvfs'; import { - ERROR_FILE_OPERATION_FAILED, ERROR_DELTA_FAILED, createPushyError, getErrorMessage, @@ -66,11 +65,14 @@ export function checkOriginalSize( /** * 目标所在文件系统必须还能再写 bytesToWrite + 安全余量。取不到可用空间 - * (返回 0/异常)视为未知,放行。 + * (返回 0/异常)视为未知,放行。空间不足以 `code` 失败:错误码按阶段归类, + * 与 Android/iOS 一致——下载阶段 DOWNLOAD_FAILED,解压阶段 PATCH_FAILED。 + * 换镜像的判定(IsMirrorRetryableCode)与补丁健康度遥测都按这个码聚合。 */ export async function ensureFreeSpace( target: string, bytesToWrite: number, + code: string, ): Promise { let probe = target; while (probe.length > 1 && !fileIo.accessSync(probe)) { @@ -92,7 +94,7 @@ export async function ensureFreeSpace( const needed = Math.max(0, bytesToWrite) + FREE_DISK_MARGIN_BYTES; if (free < needed) { throw createPushyError( - ERROR_FILE_OPERATION_FAILED, + code, `insufficient disk space: need ${needed} bytes, have ${free}`, ); } diff --git a/harmony/pushy/src/main/ets/DownloadTask.ts b/harmony/pushy/src/main/ets/DownloadTask.ts index 6b6fc6f4..27dc3b9b 100644 --- a/harmony/pushy/src/main/ets/DownloadTask.ts +++ b/harmony/pushy/src/main/ets/DownloadTask.ts @@ -286,7 +286,7 @@ export class DownloadTask { } const originalSize = await this.readOriginalSize(archiveFile); checkUncompressedSize(archiveStat.size, originalSize); - await ensureFreeSpace(unzipDirectory, originalSize); + await ensureFreeSpace(unzipDirectory, originalSize, ERROR_DELTA_FAILED); try { await zlib.decompressFile(archiveFile, unzipDirectory); } catch (e) { @@ -569,8 +569,9 @@ export class DownloadTask { received += data.byteLength; if (!writeError && baseOffset + received > MAX_ARCHIVE_BYTES) { // 未知长度/分块传输的兜底:超过上限即停写,请求在下面结算时失败。 + // 下载阶段:与 Android/iOS 一样归为 DOWNLOAD_FAILED。 writeError = createPushyError( - ERROR_DELTA_FAILED, + ERROR_DOWNLOAD_FAILED, `archive too large: exceeded ${MAX_ARCHIVE_BYTES}`, ); } @@ -592,7 +593,11 @@ export class DownloadTask { } try { if (probeFreeSpace) { - await ensureFreeSpace(params.targetFile, reserve); + await ensureFreeSpace( + params.targetFile, + reserve, + ERROR_DOWNLOAD_FAILED, + ); } await fileIo.write(writer.fd, data); } catch (error) { @@ -828,7 +833,7 @@ export class DownloadTask { } if (totalAll > MAX_ARCHIVE_BYTES) { writeError = createPushyError( - ERROR_DELTA_FAILED, + ERROR_DOWNLOAD_FAILED, `archive too large: ${totalAll} bytes`, ); throw writeError; @@ -836,6 +841,7 @@ export class DownloadTask { await ensureFreeSpace( params.targetFile, totalAll > 0 ? totalAll - baseOffset : 0, + ERROR_DOWNLOAD_FAILED, ); // 先落 sidecar 再放行写入,流中崩溃才有得续。 await this.writeResumeMeta(params.targetFile, buildResumeMeta(totalAll)); diff --git a/harmony/pushy/src/main/ets/UpdateContext.ts b/harmony/pushy/src/main/ets/UpdateContext.ts index 6bd8aa0d..7cefc973 100644 --- a/harmony/pushy/src/main/ets/UpdateContext.ts +++ b/harmony/pushy/src/main/ets/UpdateContext.ts @@ -29,6 +29,7 @@ import NativePatchCore, { import { assertSafePathComponent } from './PathUtils'; import { withRawFd } from './SaveFile'; import { + createPushyError, ERROR_SWITCH_VERSION_FAILED, getErrorMessage, toPushyError, @@ -411,6 +412,60 @@ export class UpdateContext { // switchVersion 激活。 private taskChain: Promise = Promise.resolve(); + // 在途的破坏性清理(常规清理与 reset 清理)及其开始次数。激活与破坏性清理 + // 互斥(Android 的 versionFilesLock):激活校验前先等在途清理结束,校验后 + // 若期间又有清理开始则重来。清理的保留名单在任务开始时(主线程)取,而 + // 校验通过到 applySwitch 之间没有 await,所以此后开始的清理一定看得到 + // 刚激活的版本。 + private cleanupInFlight: Promise | null = null; + private cleanupStarts: number = 0; + + private enqueueCleanupTask(job: () => Promise): Promise { + return this.enqueueSerialTask(() => { + this.cleanupStarts += 1; + const running = job(); + const tracked = running.then( + () => undefined, + () => undefined, + ); + this.cleanupInFlight = tracked; + tracked.then(() => { + if (this.cleanupInFlight === tracked) { + this.cleanupInFlight = null; + } + }); + return running; + }); + } + + private static readonly MAX_ACTIVATION_VERIFY_ATTEMPTS = 3; + + /** + * verifyActivation,并保证返回时没有破坏性清理在途、校验期间也没有清理 + * 开始过(见 cleanupInFlight)。调用方须在返回后、下一个 await 之前完成 + * applySwitch。 + */ + private async verifyActivationExclusive(hash: string): Promise { + for ( + let attempt = 0; + attempt < UpdateContext.MAX_ACTIVATION_VERIFY_ATTEMPTS; + attempt++ + ) { + while (this.cleanupInFlight !== null) { + await this.cleanupInFlight; + } + const starts = this.cleanupStarts; + await this.verifyActivation(hash); + if (this.cleanupInFlight === null && this.cleanupStarts === starts) { + return; + } + } + throw createPushyError( + ERROR_SWITCH_VERSION_FAILED, + `Version ${hash} kept racing cleanup while switching.`, + ); + } + private enqueueSerialTask(job: () => Promise): Promise { const run = this.taskChain.then(job); this.taskChain = run.then( @@ -589,7 +644,7 @@ export class UpdateContext { // maxAgeDays=0：删除下载目录内容，仅保留当前运行版本的目录（残留目录由 // 下次常规清理回收）。挂到串行任务链尾，避免与在飞的解压/打补丁并发。 - this.enqueueSerialTask(() => + this.enqueueCleanupTask(() => NativePatchCore.cleanupOldEntries( this.rootDir, [UpdateContext.launchVersion].filter(name => name.length > 0), @@ -626,7 +681,7 @@ export class UpdateContext { } const shouldSwitch = activate && !!hash; if (shouldSwitch) { - await this.verifyActivation(hash); + await this.verifyActivationExclusive(hash); if (UpdateContext.resetGeneration !== expectedGeneration) { return false; } @@ -757,7 +812,16 @@ export class UpdateContext { public async switchVersion(hash: string): Promise { try { const safeHash = assertSafePathComponent(hash); - await this.verifyActivation(safeHash); + const generation = UpdateContext.resetGeneration; + await this.verifyActivationExclusive(safeHash); + // 与 commitSyncResult 一致:校验期间发生过 reset(它会删掉刚校验过的 + // 目录),不能再把下次启动指向它。 + if (UpdateContext.resetGeneration !== generation) { + throw createPushyError( + ERROR_SWITCH_VERSION_FAILED, + `Reset to the packaged bundle while switching to ${safeHash}.`, + ); + } this.applySwitch(safeHash); await this.flushPreferences(`switch version ${safeHash}`); } catch (e) { @@ -872,19 +936,21 @@ export class UpdateContext { * executor)。 */ public cleanUp(): void { - const state = this.getStateSnapshot(); - const keepNames = [ - state.currentVersion || '', - state.lastVersion || '', - UpdateContext.launchVersion, - ].filter(name => name.length > 0); - this.enqueueSerialTask(() => - NativePatchCore.cleanupOldEntries( + // 保留名单在任务真正开始时取,不在入队时取(与 Android 一致):清理可能 + // 排在一个长下载后面,期间激活的版本必须在名单里。 + this.enqueueCleanupTask(() => { + const state = this.getStateSnapshot(); + const keepNames = [ + state.currentVersion || '', + state.lastVersion || '', + UpdateContext.launchVersion, + ].filter(name => name.length > 0); + return NativePatchCore.cleanupOldEntries( this.rootDir, keepNames, CLEANUP_MAX_AGE_DAYS, - ), - ).catch((error: Object) => { + ); + }).catch((error: Object) => { logger.error(TAG, `cleanupOldEntries failed: ${getErrorMessage(error)}`); }); } diff --git a/ios/RCTPushy/RCTPushyInstallRecord.mm b/ios/RCTPushy/RCTPushyInstallRecord.mm index eacb8c2d..8591b99f 100644 --- a/ios/RCTPushy/RCTPushyInstallRecord.mm +++ b/ios/RCTPushy/RCTPushyInstallRecord.mm @@ -1,8 +1,8 @@ // Version directories and the completion record: where an install lands and // how it is recorded as complete. The record itself (format, atomic write, // presence and activation checks) is the shared core's -// (cpp/patch_core/install_record.h); switchVersion:error: calls its -// activation check directly. +// (cpp/patch_core/install_record.h); every activation goes through its +// activation check (PushyVerifyActivation below). #import "RCTPushyInternal.h" #include "../../cpp/patch_core/error_codes.h" #include "../../cpp/patch_core/install_record.h" @@ -59,3 +59,43 @@ PUSHY_HIDDEN BOOL PushyHasCompletedVersionAtPath(NSString *versionDir, NSString } return nil; } + +// Versions activated before records existed are grandfathered through the +// persisted current/last state, but only while their directory has no record +// file (install_record::CheckActivation). Call under the state lock. +static BOOL PushyIsCurrentOrLastLocked(const std::string &hash) { + pushy::state::State state = PushyStateFromDefaults(PushyDefaults()); + return state.current_version == hash || state.last_version == hash; +} + +PUSHY_HIDDEN NSError *PushyVerifyActivation(NSString *hash, PushyVerifiedActivation *out) { + if (!PushyIsSafePathComponent(hash)) { + return PushyErrorWithCode(pushy::error_codes::kInvalidOptions, ERROR_OPTIONS); + } + PushyVerifiedActivation verified; + verified.versionDir = PushyToStdString(PushyVersionDir(hash)); + verified.hash = PushyToStdString(hash); + __block BOOL grandfathered = NO; + const std::string hashStd = verified.hash; + PushyWithStateLock(^{ + grandfathered = PushyIsCurrentOrLastLocked(hashStd); + }); + pushy::install_record::Status status = pushy::install_record::CheckActivation( + verified.versionDir, verified.hash, PushyToStdString(BUNDLE_FILE_NAME), grandfathered, + &verified.activation); + if (!status.ok) { + return PushyErrorWithCode(pushy::error_codes::kSwitchVersionFailed, + PushyFromStdString(status.message)); + } + if (out != NULL) { + *out = verified; + } + return nil; +} + +PUSHY_HIDDEN BOOL PushyVerifiedActivationUnchangedLocked(const PushyVerifiedActivation &verified) { + return pushy::install_record::Unchanged(verified.versionDir, + PushyToStdString(BUNDLE_FILE_NAME), + PushyIsCurrentOrLastLocked(verified.hash), + verified.activation); +} diff --git a/ios/RCTPushy/RCTPushyInstaller.mm b/ios/RCTPushy/RCTPushyInstaller.mm index cf11f498..7c01a3fc 100644 --- a/ios/RCTPushy/RCTPushyInstaller.mm +++ b/ios/RCTPushy/RCTPushyInstaller.mm @@ -599,47 +599,14 @@ + (void)unzipFileAtPath:(NSString *)path + (BOOL)switchVersion:(NSString *)hash error:(NSError **)error { - if (!PushyIsSafePathComponent(hash)) { - if (error != NULL) { - *error = PushyErrorWithCode(pushy::error_codes::kInvalidOptions, ERROR_OPTIONS); - } - return NO; - } - - // Same gate as Android and Harmony (install_record::CheckActivation): - // only a version this SDK recorded as completely installed, whose - // record's bundle digest matches the bytes on disk, may be activated. - // Versions activated before records existed are grandfathered through - // current/last state, but only while their directory has no record file. - std::string versionDir = PushyToStdString(PushyVersionDir(hash)); - std::string hashStd = PushyToStdString(hash); - std::string bundleName = PushyToStdString(BUNDLE_FILE_NAME); - BOOL (^isCurrentOrLast)(void) = ^BOOL { - pushy::state::State state = PushyStateFromDefaults(PushyDefaults()); - return state.current_version == hashStd || state.last_version == hashStd; - }; - - // The digest check reads the whole bundle, so it runs OUTSIDE the state - // lock: +bundleURL takes that lock on the main thread at launch. - __block BOOL grandfathered = NO; - PushyWithStateLock(^{ - grandfathered = isCurrentOrLast(); - }); - pushy::install_record::Activation activation; - pushy::install_record::Status verified = pushy::install_record::CheckActivation( - versionDir, hashStd, bundleName, grandfathered, &activation); - - __block NSError *lockedError = verified.ok - ? nil - : PushyErrorWithCode(pushy::error_codes::kSwitchVersionFailed, - PushyFromStdString(verified.message)); - if (lockedError == nil) { + // Same gate as Android and Harmony (install_record::CheckActivation); see + // PushyVerifyActivation. + PushyVerifiedActivation verified; + __block NSError *switchError = PushyVerifyActivation(hash, &verified); + if (switchError == nil) { PushyWithStateLock(^{ - // Re-check cheaply that the directory is still the one that was - // verified: same grandfathering, same record, same bundle file. - if (!pushy::install_record::Unchanged(versionDir, bundleName, isCurrentOrLast(), - activation)) { - lockedError = PushyErrorWithCode( + if (!PushyVerifiedActivationUnchangedLocked(verified)) { + switchError = PushyErrorWithCode( pushy::error_codes::kSwitchVersionFailed, [NSString stringWithFormat:@"Bundle version %@ changed while switching.", hash]); return; @@ -647,9 +614,9 @@ + (BOOL)switchVersion:(NSString *)hash error:(NSError **)error PushySwitchVersionLocked(hash); }); } - if (lockedError != nil) { + if (switchError != nil) { if (error != NULL) { - *error = lockedError; + *error = switchError; } return NO; } diff --git a/ios/RCTPushy/RCTPushyInternal.h b/ios/RCTPushy/RCTPushyInternal.h index 21835857..21bb0249 100644 --- a/ios/RCTPushy/RCTPushyInternal.h +++ b/ios/RCTPushy/RCTPushyInternal.h @@ -21,6 +21,7 @@ #include #include +#include "../../cpp/patch_core/install_record.h" #include "../../cpp/patch_core/state_core.h" // RN's log macros pass __FILE__, which would embed this file's absolute build @@ -87,6 +88,26 @@ PUSHY_HIDDEN BOOL PushyHasCompletedVersionAtPath(NSString *versionDir, NSString PUSHY_HIDDEN NSString *PushyRecordedBundleSha256(NSString *versionDir); PUSHY_HIDDEN NSError *PushyPromoteStagingDir(NSString *hash, NSString *artifactSha256); +// The activation gate every path that points the next launch at a version +// goes through (switchVersion:error:, the native round's commit and the crash +// hold's pending activation), as Android and Harmony do: only a version +// recorded as completely installed, whose record's bundle digest matches the +// bytes on disk, may be activated (install_record::CheckActivation). Split in +// two because the digest reads the whole bundle and must not run under the +// state lock (+bundleURL takes it on the main thread at launch). +struct PushyVerifiedActivation { + std::string versionDir; + std::string hash; + pushy::install_record::Activation activation; +}; +// Outside the state lock. nil when `hash` may be activated (and *out holds +// what was verified), else a SWITCH_VERSION_FAILED error. +PUSHY_HIDDEN NSError *PushyVerifyActivation(NSString *hash, PushyVerifiedActivation *out); +// Under the state lock, right before PushySwitchVersionLocked: whether the +// directory is still the one verified (same grandfathering, same record, same +// bundle file). No hashing. +PUSHY_HIDDEN BOOL PushyVerifiedActivationUnchangedLocked(const PushyVerifiedActivation &verified); + #pragma mark - RCTPushyInstaller.mm // Protocol key of the artifact URL in download options. diff --git a/ios/RCTPushy/RCTPushyOrchestrator.mm b/ios/RCTPushy/RCTPushyOrchestrator.mm index 67b0972a..a7a927d3 100644 --- a/ios/RCTPushy/RCTPushyOrchestrator.mm +++ b/ios/RCTPushy/RCTPushyOrchestrator.mm @@ -617,10 +617,9 @@ + (void)activatePendingVersion { if (hash == nil) { return; } - // Unlike Android's switchVersion, PushySwitchVersionLocked does not - // validate the version directory; never point the next launch at bytes - // that are gone (e.g. wiped by a reset whose generation bump we would - // catch below anyway, or by cleanup). + // Cheap early out for bytes that are gone (e.g. wiped by a reset whose + // generation bump the commit would catch anyway, or by cleanup); the + // commit itself runs the full activation gate (PushyVerifyActivation). if (!PushyHasCompletedVersionAtPath(PushyVersionDir(hash), hash)) { NSLog(@"RCTPushy -- crash hold: version %@ no longer on disk, dropping activation", hash); return; @@ -633,17 +632,22 @@ + (void)activatePendingVersion { merged[PushyInfoCrashHold()] = @YES; hashInfoEntry = @{@"hash": hash, @"info": merged}; } + NSError *verifyError = nil; BOOL committed = [self commitRoundWithGeneration:generation configGeneration:configGeneration hashInfo:hashInfoEntry activate:hash cache:nil - activated:NULL]; + activated:NULL + verifyError:&verifyError]; if (committed) { @synchronized (self) { pushyUnactivatedHash = nil; } NSLog(@"RCTPushy -- crash hold: activated downloaded version %@", hash); + } else if (verifyError != nil) { + NSLog(@"RCTPushy -- crash hold: version %@ failed verification, dropping activation: %@", + hash, verifyError.localizedDescription); } else { NSLog(@"RCTPushy -- crash hold: reset or new config since download, dropping activation"); } @@ -747,7 +751,8 @@ + (void)runConfiguredRound:(NSString *)configJson hashInfo:nil activate:nil cache:cache - activated:NULL]; + activated:NULL + verifyError:NULL]; return committed ? result : cancelledByReset; }; @@ -794,13 +799,23 @@ + (void)runConfiguredRound:(NSString *)configJson BOOL activate = decision.Get("activate").Truthy() || pushyCrashHoldActive.load() || pushyPurgeRestoreActive.load(); BOOL activated = NO; + NSError *verifyError = nil; BOOL committed = [self commitRoundWithGeneration:resetGeneration configGeneration:configGeneration hashInfo:@{@"hash": hash, @"info": versionInfo} activate:activate ? hash : nil cache:cache - activated:&activated]; + activated:&activated + verifyError:&verifyError]; activate = activated; + if (verifyError != nil) { + // Android parity (UpdateContext.commitSyncResult): a version that + // fails the activation gate commits nothing, not even the cache. + RCTLogWarn(@"RCTPushy -- native sync: %@ failed verification: %@", hash, + verifyError.localizedDescription); + pushyHostRoundResult = PushyHostResult(@"failed", @"commit_failed"); + return; + } if (!committed) { RCTLogInfo(@"RCTPushy -- native sync: reset during round, dropping result"); } else if (activate) { @@ -828,16 +843,35 @@ + (void)runConfiguredRound:(NSString *)configJson // reset and config generations. resetToPackagedBundle bumps that generation under the same // lock, so there is no compare-and-act window: either the whole round commits, // or the reset wins and none of it does. +// +// A version to activate first passes the same gate as switchVersion:error: +// (PushyVerifyActivation, outside the lock) and is re-checked unchanged under +// it. A version that fails it commits nothing: NO with *verifyError set. + (BOOL)commitRoundWithGeneration:(uint64_t)generation configGeneration:(uint64_t)configGeneration hashInfo:(NSDictionary *)hashInfoEntry activate:(NSString *)hashToActivate cache:(NSString *)cache - activated:(BOOL *)activatedOut { + activated:(BOOL *)activatedOut + verifyError:(NSError **)verifyErrorOut { // cache is nil for the crash handler's activation-only commit // (activatePendingVersion): no round ran, so there is no cache to write. + PushyVerifiedActivation verified; + __block NSError *verifyError = nil; + if (hashToActivate != nil) { + verifyError = PushyVerifyActivation(hashToActivate, &verified); + } __block BOOL committed = NO; __block BOOL activated = NO; + if (verifyError != nil) { + if (verifyErrorOut != NULL) { + *verifyErrorOut = verifyError; + } + if (activatedOut != NULL) { + *activatedOut = NO; + } + return NO; + } PushyWithStateLock(^{ if (pushyResetGeneration.load() != generation) { return; @@ -864,6 +898,12 @@ + (BOOL)commitRoundWithGeneration:(uint64_t)generation activation = nil; } } + if (activation != nil && !PushyVerifiedActivationUnchangedLocked(verified)) { + verifyError = PushyErrorWithCode( + pushy::error_codes::kSwitchVersionFailed, + [NSString stringWithFormat:@"Bundle version %@ changed while switching.", activation]); + return; + } NSUserDefaults *defaults = PushyDefaults(); NSString *infoJson = hashInfoEntry == nil ? nil : PushyJsonString(info); if (infoJson != nil) { @@ -881,6 +921,9 @@ + (BOOL)commitRoundWithGeneration:(uint64_t)generation if (activatedOut != NULL) { *activatedOut = activated; } + if (verifyErrorOut != NULL) { + *verifyErrorOut = verifyError; + } return committed; } diff --git a/scripts/test-ios-purge-restore.sh b/scripts/test-ios-purge-restore.sh index 70fc487b..b6b32925 100755 --- a/scripts/test-ios-purge-restore.sh +++ b/scripts/test-ios-purge-restore.sh @@ -96,7 +96,8 @@ if [ "${VERIFY_REGRESSIONS:-0}" = "1" ]; then done for pair in late-activation:late_commit skip-reresolve:commit_before_signal ignore-reset:reset_wins \ - trust-digestless:install_record_activation grandfather-recorded:switch_version_records; do + trust-digestless:install_record_activation grandfather-recorded:switch_version_records \ + skip-commit-verify:round_commit_verifies; do variant="${pair%:*}" testcase="${pair#*:}" build "$variant" @@ -111,6 +112,7 @@ if [ "${VERIFY_REGRESSIONS:-0}" = "1" ]; then ignore-reset) expected="stale generation must reject ALL commit writes" ;; trust-digestless) expected="digest-less record must not activate" ;; grandfather-recorded) expected="recorded current version must be verified" ;; + skip-commit-verify) expected="a version failing verification must not be activated by the round commit" ;; esac if ! grep -F "[FAIL] $testcase: $expected" "$logfile"; then cat "$logfile" >&2 diff --git a/scripts/tests/ios-purge-restore/README.md b/scripts/tests/ios-purge-restore/README.md index 18acff13..ed09803d 100644 --- a/scripts/tests/ios-purge-restore/README.md +++ b/scripts/tests/ios-purge-restore/README.md @@ -33,7 +33,8 @@ python3 -m unittest discover -s scripts/tests/ios-purge-restore -p test_extract. declarations never match). It copies the exact definitions (with `#line` locations) of `bundleURL`, `resolveLaunchBundleURL`, `restorePurgedLaunch`, `commitRoundWithGeneration`, `resetToPackagedBundle`, their state/defaults -helpers, `+[RCTPushyInstaller switchVersion:error:]` and the relevant globals +helpers, the activation gate both share (`PushyVerifyActivation`), +`+[RCTPushyInstaller switchVersion:error:]` and the relevant globals into build-only `.inc` files, and copies the shared completion record core (`cpp/patch_core/install_record.cpp`, whose `CheckActivation`/`Unchanged` switchVersion calls) into the build directory. These are compiled unchanged @@ -106,7 +107,7 @@ since process start. ## Negative controls -`VERIFY_REGRESSIONS=1` additionally compiles five generated-only mutants: +`VERIFY_REGRESSIONS=1` additionally compiles six generated-only mutants: - Remove the closed-window activation veto: `late_commit` must fail. - Return `NO` after timeout instead of re-resolving: `commit_before_signal` must fail. @@ -115,6 +116,9 @@ since process start. `install_record.cpp`): `install_record_activation` must fail. - Grandfather current/last versions that do carry a record (same copy): `switch_version_records` must fail. +- Activate from the round commit without the activation gate (both the + out-of-lock verification and the under-lock re-check): + `round_commit_verifies` must fail. Each mutant must compile and fail its specific state assertion; unrelated compiler errors, crashes or deadlock-guard failures do not count as detection. diff --git a/scripts/tests/ios-purge-restore/extract.py b/scripts/tests/ios-purge-restore/extract.py index b0dd51fc..2c8ef64e 100644 --- a/scripts/tests/ios-purge-restore/extract.py +++ b/scripts/tests/ios-purge-restore/extract.py @@ -102,15 +102,19 @@ def declaration(source: str, name: str) -> tuple[int, str]: # What switchVersion:error: needs around the shared record core # (cpp/patch_core/install_record.cpp, compiled from CORE_SOURCE). 'PushyVersionDir', 'PushyStringIsBlank', 'PushyIsSafePathComponent', + # The activation gate switchVersion:error: and the round commit share. + 'PushyIsCurrentOrLastLocked', 'PushyVerifyActivation', + 'PushyVerifiedActivationUnchangedLocked', ] # Emitted ahead of HELPERS, which use them. -STRUCTS: list[str] = [] +STRUCTS: list[str] = ['PushyVerifiedActivation'] # The completion record core switchVersion:error: calls. Copied (with #line) # into the build directory so the record mutants below can alter it; the # test script compiles the copy instead of the checkout's file. CORE_SOURCE = Path('cpp/patch_core/install_record.cpp') CORE_MUTATIONS = ('trust-digestless', 'grandfather-recorded') -MUTATIONS = ('late-activation', 'skip-reresolve', 'ignore-reset') + CORE_MUTATIONS +MUTATIONS = ('late-activation', 'skip-reresolve', 'ignore-reset', + 'skip-commit-verify') + CORE_MUTATIONS def mutate(text: str, mutation: str) -> str: @@ -128,11 +132,22 @@ def mutate(text: str, mutation: str) -> str: # Grandfather current/last versions even when they carry a record. 'grandfather-recorded': ( r'if \(grandfathered && state == Record::State::kAbsent\)', 'if (grandfathered)'), + # Activate from the round commit without the activation gate (both + # the out-of-lock verification and the under-lock re-check). + 'skip-commit-verify': [ + (r'if \(hashToActivate != nil\) \{', 'if (false) {'), + (r'if \(activation != nil && !PushyVerifiedActivationUnchangedLocked\(verified\)\)', + 'if (false)'), + ], } - pattern, replacement = patterns[mutation] - mutated, count = re.subn(pattern, replacement, text) - if count != 1: - raise ValueError(f'{mutation}: expected one mutation site, found {count}') + steps = patterns[mutation] + if isinstance(steps, tuple): + steps = [steps] + mutated = text + for pattern, replacement in steps: + mutated, count = re.subn(pattern, replacement, mutated) + if count != 1: + raise ValueError(f'{mutation}: expected one mutation site, found {count}') return mutated diff --git a/scripts/tests/ios-purge-restore/purge_restore_test.mm b/scripts/tests/ios-purge-restore/purge_restore_test.mm index 5496d7e8..888f33fa 100644 --- a/scripts/tests/ios-purge-restore/purge_restore_test.mm +++ b/scripts/tests/ios-purge-restore/purge_restore_test.mm @@ -189,7 +189,8 @@ + (BOOL)commitRoundWithGeneration:(uint64_t)generation hashInfo:(NSDictionary *)hashInfo activate:(NSString *)hash cache:(NSString *)cache - activated:(BOOL *)activated; + activated:(BOOL *)activated + verifyError:(NSError **)verifyError; @end #include "helpers.inc" @@ -298,7 +299,8 @@ + (void)startRoundWithDeadline:(NSTimeInterval)deadline { roundCommitted = [self commitRoundWithGeneration:requestGeneration configGeneration:pushyNativeConfigGeneration.load() hashInfo:@{@"hash": @"B", @"info": @{@"name": @"B", @"forceBootRescue": @YES}} - activate:@"B" cache:@"{\"ts\":123,\"body\":\"{}\"}" activated:&roundActivated]; + activate:@"B" cache:@"{\"ts\":123,\"body\":\"{}\"}" activated:&roundActivated + verifyError:NULL]; pushyHostRoundResult = @{@"status": @"test", @"reason": @"", @"hash": @"B"}; Signal(commitFinished); actor = Actor::None; @@ -348,6 +350,11 @@ static void SetUp(Order selected) { withIntermediateDirectories:YES attributes:nil error:NULL], "create B fixture directory"); Expect([@"B" writeToFile:[versionDir stringByAppendingPathComponent:BUNDLE_FILE_NAME] atomically:YES encoding:NSUTF8StringEncoding error:NULL], "create B fixture bundle"); + // B was installed like any download: sealed with its completion record, + // so the round commit's activation gate (PushyVerifyActivation) passes. + Expect(pushy::install_record::Seal(PushyToStdString(versionDir), "B", + PushyToStdString(BUNDLE_FILE_NAME), "", "").status.ok, + "seal B fixture record"); Expect([@"packaged" writeToURL:[RCTPushy binaryBundleURL] atomically:YES encoding:NSUTF8StringEncoding error:NULL], "create packaged fixture bundle"); PushyWithStateLock(^{ @@ -684,6 +691,43 @@ static void SwitchVersionRecords(void) { Expect([NSFileManager.defaultManager removeItemAtPath:root error:NULL], "remove fixture files"); } +// The round commit (and the crash hold's activation through it) passes the +// same activation gate as switchVersion:error:. A version whose bundle no +// longer matches its record commits nothing at all, as on Android: no +// activation, no version info, no response cache. +static void RoundCommitVerifies(void) { + SetUp(Order::CloseFirst); + NSString *bundle = [[[RCTPushy downloadDir] stringByAppendingPathComponent:@"B"] + stringByAppendingPathComponent:BUNDLE_FILE_NAME]; + Expect([@"tampered" writeToFile:bundle atomically:YES encoding:NSUTF8StringEncoding error:NULL], + "tamper with B bundle"); + BOOL activated = YES; + NSError *error = nil; + BOOL committed = [RCTPushyOrchestrator commitRoundWithGeneration:0 + configGeneration:0 + hashInfo:@{@"hash": @"B", @"info": @{@"name": @"B"}} + activate:@"B" cache:@"{}" activated:&activated verifyError:&error]; + Expect(!committed && !activated && error != nil, + "a version failing verification must not be activated by the round commit"); + Expect([error.domain isEqualToString:@(pushy::error_codes::kSwitchVersionFailed)], + "verification failures are SWITCH_VERSION_FAILED"); + Expect(State().current_version == "A", "state must keep the previous version"); + Expect(Info() == nil, "a rejected commit writes no version info"); + Expect(Value(keyNativeCheckCache) == nil, "a rejected commit writes no response cache"); + + // The intact bundle passes the same gate. + Expect([@"B" writeToFile:bundle atomically:YES encoding:NSUTF8StringEncoding error:NULL], + "restore B bundle"); + error = nil; + committed = [RCTPushyOrchestrator commitRoundWithGeneration:0 + configGeneration:0 + hashInfo:@{@"hash": @"B", @"info": @{@"name": @"B"}} + activate:@"B" cache:@"{}" activated:&activated verifyError:&error]; + Expect(committed && activated && error == nil, "a verified version activates"); + Expect(State().current_version == "B", "state selects the verified version"); + Expect([[NSFileManager defaultManager] removeItemAtPath:root error:NULL], "remove fixture files"); +} + int main(int argc, char **argv) { @autoreleasepool { // These intentional failures run in separate processes. Erasing the @@ -716,6 +760,7 @@ int main(int argc, char **argv) { {"spent_budget_skip", [] { Skipped(NO); }}, {"install_record_activation", InstallRecordActivation}, {"switch_version_records", SwitchVersionRecords}, + {"round_commit_verifies", RoundCommitVerifies}, }; int ran = 0; for (const auto &test : cases) {