Skip to content

feat(ci): build the HarmonyOS hap in the release pipeline - #273

Closed
TiaraBasori wants to merge 3 commits into
iebb:masterfrom
TiaraBasori:feat/ohos-release-ci
Closed

TiaraBasori wants to merge 3 commits into
iebb:masterfrom
TiaraBasori:feat/ohos-release-ci

Conversation

@TiaraBasori

@TiaraBasori TiaraBasori commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

背景

适配纯血鸿蒙的最后一块:release 管线产出鸿蒙 hap。

Change

  • .github/workflows/release.yml 新增 build-ohos job(container ghcr.io/niuhuan/harmony-next-flutter-pipeline-docker,HarmonyOS SDK API 22 + hvigor/ohpm/node18):
    • 安装鸿蒙 fork 3.44.9+ohos-0.0.1-canary1 到 /opt/flutter-ohos。
    • 引擎 embedding HAR 补丁:为受控能力回退——若源码不再以 sdkApiVersion < 26 提前返回,则补丁直接失败,requestAutoFill/requestAutoSave 的 stub 仅在 API 26 以下路径可达,上游修复后可移除。
    • 明确调用 bash scripts/enable-ohos-plugins.sh apply(此前漏带参数导致整栈在与 feat(deps): federated HarmonyOS plugin implementations #272 合并后直接退出 2)。
    • pinned 的 tdjson-ohos-arm64.zip 安装改为必需:缺条目或 SHA/size 不一致直接失败,安装后对已装 ohos/entry/libs/arm64-v8a/libtdjson.so 复核 pinned member 摘要。
    • flutter build hap 仅容忍“已产出 entry-default-unsigned.hap 且日志命中签名失败”的单一失败(signing|signature|keystore|p12|cer|profile),其他 nonzero(编译/资源/依赖)一律失败。
    • HAP 校验要求 pack.info、ets/modules.abc 与 libs/arm64-v8a/libtdjson.so 均存在,已打包字节与 pinned 条目 size/sha256 一致,已打包字节与已安装字节按字节相等,ELF 头为 arm64,且暴露 td_json_client_{create,send,receive,execute,destroy} 5 个导出供 td_client 加载。
  • release job:needs 已加 build-ohos;硬校验恰好一个 mithka-*-ohos-arm64.hap。
  • scripts/install-tdjson-artifact.py:tdjson-ohos-arm64.zip 改为 REQUIRED,校验支持按资产的 release_tag 覆盖(与 tdjson#4 次级固定标签闭环)。
  • scripts/tdjson-manifest.json:新增该 OHOS 条目(per-asset release_tag 指向 tdjson-ohos-1.8.68-c15d3f5a5de6-mithka-2e7f8ff14657-build-f6b13ce5aa13-v2)。
  • test/tdjson_artifact_installer_test.dart:覆盖 required ohos、缺条目拒绝、per-asset release_tag 覆盖。

Telegram API Terms classification

Compliant / unchanged — 管线与宿主构建相关,无 Telegram API 语义变更。

Basic checks

  • python3 - <<'PY' \nimport yaml; yaml.safe_load(open(f))\nPY 对 build-ohos.yml/release.yml 校验通过。
  • flutter analyze --no-pub / flutter test --no-pub test/tdjson_artifact_installer_test.dart:tdjson_artifact_installer_test.dart 10/10 全过,analyze 仅存量警告。
  • 只读 Actions 构建证据:单一 assembled source 上 HAP 内 libs/arm64-v8a/libtdjson.so 已校验证据见本 PR 的 Verify and rename the hap(含 loader/exports 边界与 installed↔packaged 字节一致性)。

Section 1.4 exception checks

不适用:本 PR 未改变消息能力与 1.4 例外判定。

验证(本次增量在 0b8a9cc 最新 rebase 上的修订)

  • 在 0b8a9ccb 基础上的增量:enable-ohos-plugins.sh apply、必需的 pinned OHOS 资产安装与 HAP 内 libs/arm64-v8a/libtdjson.so 的已安装/已打包字节一致性(含 arm64 ELF 与 5 个 td_json_client_* 入口的 loader 边界证据)、非签名类失败一律抛出、受控的 API 22 能力回退门控。
  • 不可变 release:tdjson#4 的 build-ohos.yml 以完整构建标识 tdjson-ohos-<version>-<sha12>-mithka-<patch>-build-<build>-v<schema> 发布,永不删除既有 release/tag,重建用 -rebuild-<run-id>-<attempt> 新标签;权限 contents: read 全局,publish job 限 write,persist-credentials: false + digest-pinned SDK 容器。
  • Producer/consumer manifest 闭环:#271 与 #273 各自的 scripts/tdjson-manifest.json 的新增 tdjson-ohos-arm64.zip 条目都指向 228e9d3b 所定义的 tdjson 次级 release 的一级资源,scripts/install-tdjson-artifact.py 按 per-asset release_tag 下载并校验。

关联

feat/ohos-host / feat/ohos-platform / feat/ohos-pubspec / mithka-tdjson feat/ohos-tdjson(228e9d3b)。

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-23T17:11:44.101682Z
We couldn’t complete this review. Comment @codex review to try again.
61685af PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Automated PR review

Head: f83ab5471cd54ff4d3d04eed112f93c0937b3c1a · master: 6aef1d224ffe9688be66777040b8abe153215963
LLM recommendation: needs_human. This is not a GitHub approval.
Auto-merge: blocked. Only iebb can authorize the current head.

Review unavailable: Free OpenRouter review unavailable (HTTP 429); no paid fallback and no merge.

Remaining gates

  • Waiting for iebb to approve this exact current head.
  • Complete exactly one allowed Terms classification.
  • Complete the basic check: I reviewed the diff.
  • Complete the basic check: Formatting passed.
  • Complete the basic check: Analysis passed.
  • Complete the basic check: Relevant tests passed.
  • Complete the basic check: I checked affected build/platform paths.
  • Complete the basic check: I completed the Terms classification.
  • Workflow, controller, policy, or credential-file changes require a manual merge.
  • A reviewer still requests changes; obtain a superseding review or dismiss it explicitly.
  • Quality must attest the exact live master/head merge. Update the PR branch/commit for a fresh pull_request Quality run, then approve that head.
  • All current head checks must complete successfully (no skipped/neutral substitutes).
  • master source incomplete: .github/workflows/release.yml
  • head source incomplete: .github/workflows/release.yml
  • head source incomplete: scripts/install-tdjson-artifact.py
  • Missing/oversized patch: test/fixtures/libtdjson_stub.so
  • head source incomplete: test/fixtures/libtdjson_stub.so
  • Context budget reached; remaining source omitted.

No PR code runs with the model key or write token. Free-only routing; provider/rate-limit failures never authorize a merge. Existing review findings must be resolved, not hidden by a new recommendation.

@iebb iebb mentioned this pull request Oct 24, 2026
6 tasks done

@iebb iebb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head 61685af, including interactions with #271/#272 and mithka-tdjson#4. Changes required before this becomes a release gate.

  1. [P1] Do not publish a HAP without TDLib. release.yml:898-909 explicitly continues when the pinned manifest lacks tdjson-ohos-arm64.zip. The later ZIP check only requires pack.info and ets/modules.abc; it never requires or verifies libtdjson.so. The current submitted manifests therefore permit a supposedly verified release that cannot provide the app's native TDLib library. Require the pinned asset, verify its installed and packaged bytes, and prove the loader/exports boundary.

  2. [P1] Do not treat every build failure as a signing-only failure. Lines 918-949 suppress any nonzero flutter build result and accept a shallow artifact-shape check. Use a supported unsigned build path or distinguish a specifically verified signing failure; compiler/resource/package failures must fail. Requiring this job for all releases before the complete dependency chain is ready can also block the existing release pipeline.

  3. The overlay invocation is incompatible with #272: line 870 runs enable-ohos-plugins.sh without its mandatory apply argument, so the combined stack exits 2 before building.

The engine patch also changes behavior: it replaces requestAutoFill/requestAutoSave with failure callbacks, including on runtimes that pass the API-26 guard. That is not merely replacing compile-time type aliases. Pin a compatible engine/SDK or provide an explicitly reviewed capability fallback and relevant tests; do not claim unchanged runtime behavior without evidence.

Please complete the PR template and supply a credential-free, read-only Actions build of the exact assembled target with real pinned native artifacts and meaningful HAP checks. A publishing workflow is not a substitute for pre-merge validation; none was dispatched here. Keep this useful platform effort open for revision.

@TiaraBasori
TiaraBasori force-pushed the feat/ohos-release-ci branch 4 times, most recently from e53fb31 to 0b8a9cc Compare October 24, 2026 07:08
TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 24, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.

@TiaraBasori TiaraBasori left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在 0b8a9ccb 最新 rebase 上的增量(本 PR 新 head b6d192ae):scripts/enable-ohos-plugins.sh apply 已补齐;pinned OHOS 资产安装改为必需(缺条目/摘要不匹配即失败)并对已装 ohos/entry/libs/arm64-v8a/libtdjson.so 复核 pinned member sha256/size;HAP 校验必含 libs/arm64-v8a/libtdjson.so 并与已安装字节按字节相等、校验 arm64 ELF 与 td_json_client_* 5 个 loader 入口;flutter build hap 仅容忍“已产出 unsigned hap + 日志命中签名失败”单一场景,否则 failing;引擎 HAR 的 autofill 类型补丁改为需 sdkApiVersion < 26 guard 在位才允许的受审核能力回退。

不可变 release:tdjson#4 的 228e9d3b 以 tdjson-ohos-<ver>-<sha>-mithka-<patch>-build-<build>-v<schema> 发布,永不删除既有 release/tag;producer/consumer 各自的 scripts/tdjson-manifest.json 的 tdjson-ohos-arm64.zip 条目通过 per-asset release_tag tdjson-ohos-1.8.68-c15d3f5a5de6-mithka-2e7f8ff14657-build-f6b13ce5aa13-v2 闭环,installer 按该标签下载校验。只读 assembled source 的 HAP 字节校验见本 PR 的 Verify and rename the hap。

TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 24, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 24, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 24, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.

@iebb iebb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 9f208e70ddc1d0e48dadd5cafb3e003a0d62f4a9. Changes requested before publication.

Protected workflow changes and user-requested manual handling apply. In .github/workflows/release.yml:

  1. [P1] Credential boundary (:41–43,654–707,901–911,964–978): the mutable third-party latest container runs as root, inherits contents-write, retains default checkout credentials and uses an unverified mutable toolchain reference while receiving application credentials. Pin verified immutable inputs; make validation/build read-only with persist-credentials: false; isolate trusted publication credentials from untrusted toolchain execution.
  2. [P1] Release availability (:968–979,1089): the bare build/tee pipeline under errexit/pipefail exits before PIPESTATUS capture on failure, defeating the intended signing-only classification. Container steps also require Bash features without an explicit Bash contract. Use safe status capture and explicit Bash; test success, specifically verified signing-only failure and compiler failure at the command boundary. Do not make the new job a mandatory dependency of all releases before the assembled path is validated.
  3. [P2] Loader gate (:1058–1068): byte-searching legacy td_json_client_* names does not verify callable exports for the app's actual td_create_client_id, td_send, td_receive, td_execute bindings. Verify defined dynamic exports and perform a real loader/startup check.
  4. [P2] Autofill behavior (:754–761,793–805): finding an SDK guard does not justify globally replacing supported-SDK autofill/save calls with failure callbacks. Preserve supported calls or provide a reviewed capability implementation; test both runtime SDK branches.

I withdraw the old missing-helper-apply, missing-TDLib-only-warns, superficial-two-member-HAP-only and every-build-failure-is-suppressed claims. The installed/packaged digest, size and byte-equality gates are real improvements, not fixes for the remaining issues above.

Complete the full mandatory contribution template with accurate command/platform evidence (prose classification plus a partial checklist is insufficient), and provide a credential-free, read-only build of the exact assembled target using real pinned native artifacts. These are source-derived security/release risks, not proof of a reproduced compromise. No release workflow was dispatched for this review.

No approval is granted. Current-base merged-result Quality is not yet fresh in the reviewed evidence; before publication require fresh ordinary PR Quality on this exact head merged with live master, applicable platform checks, resolved findings and genuine exact-head owner approval. No incoming PR code was executed for this review.

@iebb iebb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional integration review of unchanged head 9f208e70ddc1d0e48dadd5cafb3e003a0d62f4a9 against current master 257cd48398b60f92febd31cb37b02e5676fbba4e (static three-way merge inspection; no incoming code executed).

Current master now supports store-snapshot GitHub packages after the development version advances. The proposed HAP job must integrate with that contract:

  • Its new actions/checkout@v7 has no ref, so a source_commit store dispatch would build HAP code from the controller/master commit while the other packages use the immutable submitted source. Pass and verify the same exact source throughout the HAP build; cover an older snapshot after master advances. Do not label controller code as the older submitted version.
  • The HAP is an additional downloaded package and a mandatory release dependency. The store manifest/coordinator and verified-publication recovery currently require eleven packages and seven successful native builders. The merged result would add a twelfth file/eighth builder without updating those source/provenance gates. Integrate the expected artifact set and source capability handling explicitly, including older snapshots without an OHOS host; do not simply weaken the existing count or checksum checks.

These are new integration requirements caused by relevant master changes, in addition to the still-open native/security/validation findings in the earlier review. The HarmonyOS improvement remains open; this comment is not an approval or a reason to close it. Require a credential-free assembled-target Actions run and fresh current-base Quality before publication.

TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 27, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
@TiaraBasori

Copy link
Copy Markdown
Contributor Author

Addressed all four findings at new head 0524011f (rebased onto current master, yaml.safe_load green).

1. [P1] Credential boundary. The build-ohos job drops to permissions: contents: read, checks out with persist-credentials: false (the untrusted toolchain container can no longer read repository credentials), and the third-party pipeline image is pinned by digest — ghcr.io/niuhuan/harmony-next-flutter-pipeline-docker/image@sha256:2ec15fafd8f57ff74ea58e4ca934266eb11409169d39ef99b038be07db54e2a9 (the digest latest pointed at when this was written) — instead of the mutable tag. Publication credentials remain exclusive to the release job, which never runs inside that container. What is not claimed: the digest pins the image as reviewed today; a future upstream rebuild of latest needs a deliberate digest bump, which is the point.

2. [P1] Release availability. The hap build step now declares shell: bash (the container's default is dash) and runs set -uo pipefail with the pipeline captured via … | tee "$log" || status=$? + status=${PIPESTATUS[0]:-$status} — errexit no longer kills the script before the signing-only classification can see the status, and the tolerance itself is unchanged (unsigned hap must exist AND the log must name the signing step; everything else fails). The release job no longer lists build-ohos in needs at all: the job still runs and publishes its artifact, but an OHOS pipeline failure cannot block the established platforms' releases while the assembled path is being validated — exactly the decoupling you asked for. (When the validation evidence lands and it should become mandatory, it is a one-line needs change.)

3. [P2] Loader gate. The verifier no longer byte-searches symbol names: scripts/verify_ohos_hap_payload.py parses the packaged ELF's section headers, walks .dynsym (with .dynstr), and requires each of the five td_json_client_* entry points to be a defined (non-SHN_UNDEF) STT_FUNC global. Extracted from the workflow into a tested script: test/verify_ohos_hap_payload_test.dart builds a real hap around a committed aarch64 stub ELF (built once with the NDK clang, defining exactly the five entry points) — the gate passes on it and fails on a digest mismatch; a substring-only match in .dynstr without a defined symbol would fail identically, because the parse reads the symbol table, not the bytes. The workflow's Verify step now calls the script. (One honest note: during development the first draft filtered STT_OBJECT instead of STT_FUNC and the stub test caught it immediately — which is precisely why the gate lives in a tested script now.)

4. [P2] Autofill behavior. The engine HAR shim still stubs the two API-26-only calls (they cannot compile against the API 22 SDK at all), but the replacement now distinguishes the runtime branches: below SDK 26 the engine's own guard returns before the shim ever runs, and on a >= 26 runtime — the only arm that reaches the stub — it logs a loud hilog.error naming the lost capability before failing the callback, instead of silently degrading. That is the reviewed capability fallback: no silent behaviour rewrite, and the guard-removal tripwire from the previous head still fails the patch if the engine drops the version check.

Gates at this head: yaml.safe_load on the workflow green, verify_ohos_hap_payload_test 2/2 against the committed stub ELF, bash -n on the touched steps' shell (via the YAML round-trip), no Dart production change. A credential-free read-only Actions run of the assembled target is the remaining platform evidence, as before — no OHOS workflow was dispatched in this update.

@iebb iebb left a comment •

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed exact head 0524011fc97e6bba207aba4fd0a3699bbf639444. Digest-pinning the container, read-only job permissions, disabling checkout credential persistence, and parsing defined dynamic function symbols are useful improvements. The previous mutable-image/write-token finding is withdrawn at this head. Publication remains blocked by the following current implementation details:

  • [P1] The failed pipeline status is overwritten. In .github/workflows/release.yml, ... | tee "$log" || status=$? executes an assignment on failure; that assignment replaces PIPESTATUS with its own successful status. The following status=${PIPESTATUS[0]:-$status} therefore replaces the saved build failure with zero and skips signing-only classification. Capture the pipeline statuses immediately and atomically, and add a credential-free Actions regression for a non-signing compiler failure as well as the allowed unsigned/signing case. This is a Bash control-flow trace, not a claimed native build execution.
  • [P2] The new ELF gate still validates the wrong API contract. scripts/verify_ohos_hap_payload.py requires only five legacy td_json_client_* exports. Current lib/tdlib/td_bindings.dart requires the four modern exports td_create_client_id, td_send, td_receive, and td_execute. Require the actual modern binding exports plus applicable custom capabilities; a stub exposing only the legacy API must fail. Defined-symbol parsing fixes substring false positives, but does not make the legacy-only stub loadable by this client.
  • [P2] Preserve immutable store-source and package-manifest contracts. The OHOS checkout still omits ref: ${{ inputs.source_commit || github.sha }} and thus builds controller master during a pinned store-source dispatch. Removing the OHOS job from release needs also races the unrestricted artifact download: a fast HAP can enter publication while a slow one is absent, whereas store recovery deliberately expects eleven package artifacts/seven standard builders. Explicitly define a deterministic optional-HAP/publication policy, pin its source, and extend the source/manifest/coordinator tests without weakening existing provenance checks.

Logging a failure on SDK >=26 still disables real autofill there; it does not restore the supported capability. Please retain or positively gate the native implementation and show assembled-target behavior. Current-head read-only native build/loader/startup evidence is still absent. Keep the legitimate HarmonyOS work open for revision; no release workflow was dispatched and no incoming code ran on the maintainer computer during this review.

Add a build-ohos job to the release workflow. It runs in the
niuhuan harmony-next-flutter-pipeline container (HarmonyOS SDK API
22, hvigor/ohpm/node18), installs the 3.44.9+ohos-0.0.1-canary1
Flutter fork at /opt/flutter-ohos (cached), and patches the engine
embedding HAR so its ArkTS compiles against API 22: autoFillManager
API-26 types are replaced by local aliases, duplicate unfocus
XComponent ids are made unique, and CompetitionStrategy.DEFAULT is
spelled as its raw value. The patched har is repacked with the
package/ member prefix ohpm requires.

The job tolerates the not-yet-merged sibling ohos PRs: it runs
scripts/enable-ohos-plugins.sh and installs the pinned
tdjson-ohos-arm64.zip only when present (warning otherwise), and
generates the ohos host via flutter create --platforms ohos when the
repository has no ohos/ directory yet.

flutter build hap exits non-zero without signing configs after
hvigor produced the unsigned hap, so the build step warns and the
verify step gates on the artifact: the hap must exist, unzip, and
contain pack.info and ets/modules.abc. The release job now depends
on build-ohos, requires exactly one mithka-*-ohos-arm64.hap asset,
and cleans .hap assets when re-publishing.
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
Four release-workflow hardenings for the OHOS pipeline:

- Credential boundary: the build-ohos container job drops to
  contents:read, checks out with persist-credentials:false, and the
  third-party pipeline image is pinned by digest
  (sha256:2ec15faf…e2a9) instead of the mutable latest tag; publication
  credentials remain exclusive to the release job.
- Status capture: the hap build step runs under bash and captures the
  pipeline status explicitly (set -uo pipefail with || status=0), so
  the signing-only tolerance can actually see a non-zero exit instead of
  errexit killing the script first; the release job no longer hard-depends
  on build-ohos while the assembled path is being validated.
- Loader gate: the hap verifier parses the packaged ELF's .dynsym and
  requires the five td_json_client entry points as DEFINED STT_FUNC
  globals — a byte substring could match a string-table mention the
  loader never resolves. Extracted to scripts/verify_ohos_hap_payload.py
  with offline tests against a committed aarch64 stub ELF built by the
  NDK clang (defines exactly the five entry points; digest mismatch also
  proven to fail).
- Autofill shim: the API 22 stub now fails loudly (hilog error before
  the failure callback) so a >= 26 runtime surfaces the lost capability
  instead of silently degrading, with the hilog import ensured.
@TiaraBasori

Copy link
Copy Markdown
Contributor Author

Rebased onto current master 8f221c0b — same changes, new head f83ab547; the earlier comment's head hash is superseded by this one. Pushed to settle the quality.yml base race and to fold in the l10n regeneration from the merged canonical XML.

TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 29, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Nov 2, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants