From c1cfa263b694a4a6fc6952f72a72bd59d3131244 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 06:33:17 +0800 Subject: [PATCH 01/12] ci: pin every action to a full commit SHA --- .github/workflows/android.yml | 16 ++++++++-------- .github/workflows/ci.yml | 6 +++--- .github/workflows/ios.yml | 12 ++++++------ .github/workflows/release.yml | 22 +++++++++++----------- .github/workflows/review.yml | 2 +- 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index de4c6d32c..28d9301f6 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -20,13 +20,13 @@ jobs: android: ${{ steps.filter.outputs.android }} release: ${{ steps.release.outputs.builds }} steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: # A push reads its own commit messages below: full history, without # the file contents nothing here looks at. fetch-depth: ${{ github.event_name == 'push' && '0' || '1' }} filter: ${{ github.event_name == 'push' && 'blob:none' || '' }} - - uses: dorny/paths-filter@v4 + - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3 id: filter with: filters: | @@ -63,10 +63,10 @@ jobs: contents: read steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 - name: Setup Java - uses: actions/setup-java@v6 + uses: actions/setup-java@b843ebf9b913146b38da2eaa7aeddf76074ae1ad # v6.1.0 with: distribution: "temurin" java-version: "25" @@ -78,10 +78,10 @@ jobs: # built against an SDK nobody chose, and nothing in the build log would # say so. See AGENTS.md → Toolchain. - name: Install toolchain (mise) - uses: jdx/mise-action@v4 + uses: jdx/mise-action@c2a87611a18de5b3828c5652fe268e992400cb5c # v4.3.0 - name: Cache Android NDK - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.1.0 with: path: | /usr/local/lib/android/sdk/ndk/27.0.12077973 @@ -89,7 +89,7 @@ jobs: key: ${{ runner.os }}-ndk-27-28 - name: Cache Android sdk - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.1.0 env: cache-name: cache-Android-sdk with: @@ -123,7 +123,7 @@ jobs: run: bash tool/dev/build.sh android - name: Upload Artifacts - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: DPIP-Android-Release path: build/app/outputs/flutter-apk/*.apk diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e65c9aacb..37254b073 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,7 @@ jobs: steps: # Full history: the commit gate walks the range this push or PR adds, and # a shallow clone has no base to compare against. - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: fetch-depth: 0 @@ -138,7 +138,7 @@ jobs: # developers run locally — one source of truth. v4 = Node 24 runtime # (Node 20 actions are forced onto 24 and emit deprecation warnings). - name: Install toolchain (mise) - uses: jdx/mise-action@v4 + uses: jdx/mise-action@c2a87611a18de5b3828c5652fe268e992400cb5c # v4.3.0 - name: Install dependencies run: bash tool/dev/deps.sh @@ -199,7 +199,7 @@ jobs: # tokenless opt-out for public repositories, in the Codecov org settings. - name: Upload coverage to Codecov if: ${{ !cancelled() && hashFiles('coverage/lcov.info') != '' }} - uses: codecov/codecov-action@v5 + uses: codecov/codecov-action@0fb7174895f61a3b6b78fc075e0cd60383518dac # v5.5.5 with: use_oidc: true files: coverage/lcov.info diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index a68e3c1e3..3c3c36ea2 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -32,13 +32,13 @@ jobs: ios: ${{ steps.filter.outputs.ios }} release: ${{ steps.release.outputs.builds }} steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: # A push reads its own commit messages below: full history, without # the file contents nothing here looks at. fetch-depth: ${{ github.event_name == 'push' && '0' || '1' }} filter: ${{ github.event_name == 'push' && 'blob:none' || '' }} - - uses: dorny/paths-filter@v4 + - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3 id: filter with: filters: | @@ -75,7 +75,7 @@ jobs: contents: read steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 # Flutter and Dart come from mise.toml, the same pin CI and every laptop # use. Not subosito/flutter-action with `channel: stable`: that resolves @@ -83,12 +83,12 @@ jobs: # built against an SDK nobody chose, and nothing in the build log would # say so. See AGENTS.md → Toolchain. - name: Install toolchain (mise) - uses: jdx/mise-action@v4 + uses: jdx/mise-action@c2a87611a18de5b3828c5652fe268e992400cb5c # v4.3.0 # The iOS build uses Swift Package Manager, not CocoaPods (there is no # Podfile). Cache SPM's clone/artifact store keyed on the resolved pins. - name: Cache Swift Packages - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.1.0 with: # build/ios/SourcePackages, not DerivedData. Flutter always passes # -clonedSourcePackagesDirPath pointing there (ios/xcodeproj.dart), so @@ -130,7 +130,7 @@ jobs: zip -qr DPIP.ipa Payload - name: Upload Artifacts - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: DPIP-iOS path: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2dc9915be..ba805a8a4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -37,7 +37,7 @@ jobs: steps: # Full history: the label counts this week's commits and the train reads # the newest tag. A shallow clone has neither. - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: fetch-depth: 0 @@ -156,7 +156,7 @@ jobs: runs-on: ${{ matrix.runs-on }} steps: # Full history: tool/release/build_info.sh counts commits and reads tags. - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: fetch-depth: 0 @@ -164,10 +164,10 @@ jobs: # developer use. A published build is the one that most needs to be # reproducible, so it must not be the one tracking whatever "stable" # happens to mean on the day it runs. - - uses: jdx/mise-action@v4 + - uses: jdx/mise-action@c2a87611a18de5b3828c5652fe268e992400cb5c # v4.3.0 - if: matrix.platform == 'android' - uses: actions/setup-java@v5 + uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0 with: distribution: "temurin" java-version: "25" @@ -185,7 +185,7 @@ jobs: # precise key costs one full download on a dependency change and is # correct every time. - name: Cache pub packages - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.1.0 with: path: ~/.pub-cache key: ${{ runner.os }}-pub-${{ hashFiles('pubspec.lock') }} @@ -206,7 +206,7 @@ jobs: # main saved here and stored a fresh ~880 MB copy of its own. - name: Cache Swift packages if: matrix.platform == 'ios' - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.1.0 with: path: | ~/Library/Caches/org.swift.swiftpm @@ -364,7 +364,7 @@ jobs: # can race with an operator already working in the console. - name: Upload to Play if: matrix.platform == 'android' - uses: r0adkll/upload-google-play@v1 + uses: r0adkll/upload-google-play@e738b9dd8f2476ea806d921b64aacd24f34515a5 # v1.1.5 with: serviceAccountJsonPlainText: ${{ secrets.SERVICE_ACCOUNT_JSON }} packageName: com.exptech.dpip @@ -391,7 +391,7 @@ jobs: # a release gets stuck in a state no one recognises. - name: Upload to App Store Connect if: matrix.platform == 'ios' - uses: apple-actions/upload-testflight-build@v5 + uses: apple-actions/upload-testflight-build@155ea93be9ee6d5186c71f660a612f5eddb14510 # v5.6.0 with: app-path: DPIP-${{ needs.version.outputs.label }}.ipa issuer-id: ${{ secrets.APPSTORE_ISSUER_ID }} @@ -418,7 +418,7 @@ jobs: if: always() && matrix.platform == 'ios' run: rm -rf "$RUNNER_TEMP/private_keys" - - uses: actions/upload-artifact@v6 + - uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: dpip-${{ matrix.platform }} path: DPIP-*.* @@ -430,11 +430,11 @@ jobs: permissions: contents: write steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: fetch-depth: 0 - - uses: actions/download-artifact@v7 + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: path: artifacts merge-multiple: true diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml index 1c01e4b01..e2c922cb8 100644 --- a/.github/workflows/review.yml +++ b/.github/workflows/review.yml @@ -99,7 +99,7 @@ jobs: - name: Get PR context id: pr-context if: github.event_name == 'issue_comment' - uses: actions/github-script@v8 + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 with: script: | // For issue_comment events, resolve PR base/head so the action From 1f0f22c9999010b1f3b19403e7d3223230bed037 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 06:40:34 +0800 Subject: [PATCH 02/12] ci: let Dependabot propose action updates once a week --- .github/dependabot.yml | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 000000000..a89407469 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,33 @@ +# Keeps the actions in .github/workflows current. Actions only: pub and the +# Swift packages are pinned by lockfiles a person moves on purpose, after +# reading what changed — pubspec.yaml explains why several pins must not drift. +# +# Every `uses:` is a full commit SHA with the release in a trailing comment, so +# a moved or re-pointed tag changes nothing here until a pull request says so; +# Dependabot rewrites both the SHA and the comment. +version: 2 +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + # A release is not proposed until it has been public for a week. Most + # compromised releases are caught and pulled within days, so this is the + # window in which somebody else finds out first — rather than this + # repository's release job, which holds the store signing keys. + cooldown: + default-days: 7 + # One pull request a week rather than one per action: they are reviewed + # together, and CI runs once for all of them. + groups: + github-actions: + patterns: ["*"] + # The summary in commit.md's format (`ci: bump …`). The commit as a whole + # still fails tool/check/commits.sh, on purpose: Dependabot always appends + # `Signed-off-by: dependabot[bot]`, and a commit here is authored by a + # person. Its pull request is a proposal — whoever reviews it re-commits + # it as their own (`git commit --amend --reset-author`, trailer removed) + # and force-pushes, which also makes the person who read the diff the one + # on record for it. + commit-message: + prefix: ci From 80c27b67a95739eea6e210dace0f65db41364a33 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 06:43:52 +0800 Subject: [PATCH 03/12] ci(review): keep the reviewer from running anything a PR controls --- .github/workflows/review.yml | 70 +++++++++++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 9 deletions(-) diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml index e2c922cb8..ca743ecc1 100644 --- a/.github/workflows/review.yml +++ b/.github/workflows/review.yml @@ -12,6 +12,39 @@ # # For the full list of action inputs/outputs and the four comment-posting modes # (sticky / incremental), see action.yml at the repo root. +# +# ── Threat model ──────────────────────────────────────────────────────────── +# +# This is the one workflow here that runs with secrets on a pull request from +# a fork — `pull_request_target` and `issue_comment` both run in the base +# repository's context, with its secrets and a token that can write. So it is +# the one place a stranger's code and a credential can meet, and everything +# below exists to keep them apart: +# +# - What runs is the base branch's copy of this file, never the pull +# request's. A PR cannot edit its own reviewer. +# - Nothing checks out or executes the pull request. The action checks out +# the *base* and fetches the head commit as git objects only +# (`git fetch origin pull//head`): the diff is read, never built, +# installed, sourced or run. Never add a step that checks out +# `github.event.pull_request.head.*`, runs a script or a package manager +# from the PR, or restores a cache it could have written — any of them +# turns this file into remote code execution holding the LLM key. +# - Every piece of third-party code that does run is pinned: the action to a +# commit, and the CLI it installs from npm to an exact version. The +# action's own default for that is `latest`, which would hand these +# secrets to whoever publishes the next release, unreviewed. +# - The token can read the repository and write pull-request comments, and +# nothing else (`permissions:` on the job; the workflow default is none). +# - Re-review by comment is limited to members, owners and collaborators. +# +# What it does not stop: the diff is text an LLM reads, with tools that can +# read files in the checkout, so a hostile pull request can steer what the +# review says. That is accepted — the review is advisory and blocks nothing. +# The action's own checkout persists the job token for its `git fetch`, on +# disk and within reach of those tools, which is why "Allow GitHub Actions to +# create and approve pull requests" must stay off in Settings → Actions: with +# it on, a leaked token could approve. name: OpenCodeReview PR Review @@ -54,21 +87,26 @@ concurrency: cancel-in-progress: true on: - # Use pull_request_target instead of pull_request so that secrets are - # available even for PRs from forks. This is safe because the reusable - # action only reads the diff and does not execute any code from the PR. - pull_request_target: + # pull_request_target rather than pull_request so that secrets are available + # for PRs from forks — a fork's `pull_request` run is handed none. Safe only + # as long as the threat model at the top of this file holds, which is also + # why the workflow lint is told to accept it here and nowhere else. + pull_request_target: # zizmor: ignore[dangerous-triggers] — see the threat model above types: [opened, synchronize, reopened] issue_comment: types: [created] -permissions: - contents: read - pull-requests: write +# None by default; the one job asks for exactly what it uses. +permissions: {} jobs: code-review: runs-on: ubuntu-latest + # Read the repository (the action checks out the base and fetches the + # head's objects) and post the review. Nothing else. + permissions: + contents: read + pull-requests: write # A backstop, not the cap. The review itself is capped at 15 minutes on its # own step, where a timeout can be swallowed; a job-level timeout cannot be, # and would fail the check. This only catches something wedging outside the @@ -137,8 +175,18 @@ jobs: id: review continue-on-error: true timeout-minutes: 15 - uses: alibaba/open-code-review@main + # A commit, not `main`: this code runs holding the secrets below, and a + # branch is whatever was pushed to it last. + # + # `ocr_version` is the CLI the action installs from npm, which is the + # code actually handed the LLM key. Left out, the action installs + # `latest`, and pinning the action would pin nothing. Dependabot moves + # the `uses:` line and cannot see that one, so it sits directly below, + # where a Dependabot diff shows it: bump the two together, to the npm + # release matching the action's tag. + uses: alibaba/open-code-review@fabbdb296b0d97e2140ada8d54ca7d6d6d1d7ad4 # v1.12.13 with: + ocr_version: 1.12.13 llm_url: ${{ secrets.OCR_LLM_URL }} llm_auth_token: ${{ secrets.OCR_LLM_AUTH_TOKEN }} llm_model: ${{ secrets.OCR_LLM_MODEL }} @@ -158,9 +206,13 @@ jobs: # otherwise look identical from outside: one green check either way. - name: Say so if the review did not run if: steps.review.outcome != 'success' + # Through the environment rather than spliced into the script, like + # every value in a workflow that holds secrets. + env: + OUTCOME: ${{ steps.review.outcome }} run: | printf '::warning::the code review did not complete (%s). ' \ - "${{ steps.review.outcome }}" + "$OUTCOME" printf 'This check is advisory and stays green on purpose, so nothing ' printf 'is blocked — but this pull request has not been reviewed. ' printf 'Comment /open-code-review to try again.\n' From d9db2096acb87c167c2d61826299b1d8f018dc6c Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 06:53:28 +0800 Subject: [PATCH 04/12] ci: run a pinned, checksum-verified actionlint, here and in CI --- .github/workflows/ci.yml | 11 ++-- AGENTS.md | 4 +- tool/check.sh | 8 +-- tool/check/workflows.sh | 22 ++++++++ tool/internal/pinned_tool.sh | 101 +++++++++++++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 13 deletions(-) create mode 100755 tool/check/workflows.sh create mode 100755 tool/internal/pinned_tool.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 37254b073..af79dc67d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -95,15 +95,10 @@ jobs: echo "branch gate: rebased on $BASE_REF, no merge commits" # The workflows themselves, because a broken one does not fail — it is - # *refused*. GitHub interpolates a `run` block whole before bash sees it, - # so an expression error anywhere in it, comment included, kills the run - # before a job exists: no failing step, and the file path where the name - # should be. Nothing in this repository could have caught that, and it - # took CI down on main for a day. + # *refused*: an expression error kills the run before a job exists. The + # script says more, and pins and checksums the actionlint it runs. - name: Workflow gate - run: | - bash <(curl -sSf https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash) >/dev/null - ./actionlint -shellcheck= .github/workflows/*.yml + run: bash tool/check/workflows.sh # Architecture gate — pure grep, no toolchain, fails fast. - name: Layering gate diff --git a/AGENTS.md b/AGENTS.md index 3a7ea3035..400c79b8c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -216,6 +216,7 @@ See [commit.md](commit.md). Individually, if you want to fail faster: ```sh tool/check/commits.sh origin/main..HEAD +tool/check/workflows.sh tool/check/layering.sh tool/check/l10n.sh tool/check/storage.sh @@ -229,7 +230,8 @@ tool/dev/test_ios.sh # only if you touched ios/ — macOS, and slow ``` The bash gates need only bash and python3, so they fail fast without the -toolchain. `.github/workflows/ci.yml` must stay green; +toolchain. `workflows.sh` also downloads its linter, at a pinned release with a +pinned checksum, the first time it runs in a clone. `.github/workflows/ci.yml` must stay green; `android.yml` / `ios.yml` build artifacts and `review.yml` adds an automated PR review. diff --git a/tool/check.sh b/tool/check.sh index 69fb5539f..faf318256 100755 --- a/tool/check.sh +++ b/tool/check.sh @@ -24,7 +24,6 @@ # Not run here, and on purpose: # - `pub get`. CI installs dependencies; a developer already has them, and # `check/pubspec_lock.sh` is what catches a lockfile that disagrees. -# - actionlint. CI downloads it; nothing pins it locally. # - `check/commits.sh`. It belongs to `tool/commit.sh`, which is where you # are when a commit message is the thing being decided. source "$(dirname "${BASH_SOURCE[0]}")/dev/_lib.sh" @@ -33,10 +32,11 @@ cd "$(repo_root)" step 'format + analyze' cached analyze "$(cache_key "${CODE_INPUTS[@]}")" tool/dev/analyze.sh -# Five of these finish in under a quarter of a second and are run every time: +# Most of these finish in under a quarter of a second and are run every time: # hashing their inputs would cost as much as running them, and a cache that -# saves nothing is a cache that can only be wrong. -for gate in tooling l10n pubspec_lock notification_sounds build_info \ +# saves nothing is a cache that can only be wrong. `workflows` downloads its +# linter on the first run in a clone and takes 0.06 s on every one after. +for gate in workflows tooling l10n pubspec_lock notification_sounds build_info \ widget_township_resources; do step "check/$gate" "tool/check/$gate.sh" diff --git a/tool/check/workflows.sh b/tool/check/workflows.sh new file mode 100755 index 000000000..842054b07 --- /dev/null +++ b/tool/check/workflows.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash +# The workflows themselves, because a broken one does not fail — it is refused. +# +# tool/check/workflows.sh +# +# GitHub interpolates a `run` block whole before bash sees it, so an expression +# error anywhere in it, comment included, kills the run before a job exists: +# no failing step, and the file path where the name should be. Nothing else in +# this repository could have caught that, and it took CI down on main for a +# day. +# +# actionlint, at the release tool/internal/pinned_tool.sh pins and verifies. +# Its shellcheck and pyflakes passes are off, as they have always been in CI: +# actionlint runs them only when it finds them on PATH, so leaving them on would +# make the answer depend on what the machine running it happens to have. +set -euo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")/../.." + +actionlint="$(tool/internal/pinned_tool.sh actionlint)" +"$actionlint" -shellcheck= -pyflakes= .github/workflows/*.yml + +echo "workflows OK — actionlint found nothing." diff --git a/tool/internal/pinned_tool.sh b/tool/internal/pinned_tool.sh new file mode 100755 index 000000000..73cd60539 --- /dev/null +++ b/tool/internal/pinned_tool.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# A third-party binary at a pinned release, verified before anything runs it. +# +# tool/internal/pinned_tool.sh actionlint # prints the binary's path +# +# For tools that are not the app's toolchain and so do not belong in +# mise.toml: mise-action installs everything that file lists in every workflow +# that uses it, the release job included, and a linter has no business on the +# runner that holds the store signing keys. +# +# What this replaces is `bash <(curl …/actionlint/main/scripts/…)`: a script +# fetched from a branch and executed as it arrived, so whoever could push to +# that branch could run anything on the runner, and nothing recorded which +# version had linted what. Here the release is named and the archive's SHA-256 +# is written down, and an archive that does not match is refused before it is +# unpacked. +# +# The checksums are the ones the project publishes with the release, which +# GitHub also shows as each asset's digest. To move a pin, change the version +# and replace every checksum for that tool from the new release. A version +# whose checksums were not replaced fails here; it never runs unverified. +# +# Downloaded once into the git directory, beside the gate cache in +# dev/_lib.sh and for the same reasons — nothing has to gitignore it and +# `flutter clean` cannot reach it — in a directory named by the checksum, so a +# changed pin can never pick up the previous binary. +source "$(dirname "${BASH_SOURCE[0]}")/../dev/_lib.sh" +cd "$(repo_root)" + +readonly ACTIONLINT=1.7.12 + +tool="${1:-}" +case "$tool" in +actionlint) + version=$ACTIONLINT + base="https://github.com/rhysd/actionlint/releases/download/v$ACTIONLINT" + ;; +*) + printf 'usage: tool/internal/pinned_tool.sh actionlint\n' >&2 + exit 2 + ;; +esac + +case "$(uname -s)/$(uname -m)" in +Darwin/arm64) platform=darwin-arm64 ;; +Darwin/x86_64) platform=darwin-x64 ;; +Linux/x86_64) platform=linux-x64 ;; +Linux/aarch64 | Linux/arm64) platform=linux-arm64 ;; +*) platform="$(uname -s)/$(uname -m)" ;; +esac + +# , per tool and platform. +case "$tool/$platform" in +actionlint/darwin-arm64) pin="actionlint_${ACTIONLINT}_darwin_arm64.tar.gz aba9ced2dee8d27fecca3dc7feb1a7f9a52caefa1eb46f3271ea66b6e0e6953f" ;; +actionlint/darwin-x64) pin="actionlint_${ACTIONLINT}_darwin_amd64.tar.gz 5b44c3bc2255115c9b69e30efc0fecdf498fdb63c5d58e17084fd5f16324c644" ;; +actionlint/linux-x64) pin="actionlint_${ACTIONLINT}_linux_amd64.tar.gz 8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8" ;; +actionlint/linux-arm64) pin="actionlint_${ACTIONLINT}_linux_arm64.tar.gz 325e971b6ba9bfa504672e29be93c24981eeb1c07576d730e9f7c8805afff0c6" ;; +*) + # A failure, not a skip. The gates that call this are in tool/check.sh, and + # a gate that quietly does not run turns "green here means green in CI" into + # a claim nobody checked. + printf '\n No pinned %s for %s. Add the release asset and its checksum\n' "$tool" "$platform" >&2 + printf ' to tool/internal/pinned_tool.sh, or let CI run this gate.\n\n' >&2 + exit 1 + ;; +esac +archive="${pin%% *}" +sum="${pin##* }" + +store="$(cd "$(git rev-parse --git-dir)" && pwd)/dpip-tools/$tool-${sum:0:16}" +bin="$store/$tool" +if [[ -x $bin ]]; then + printf '%s\n' "$bin" + exit 0 +fi + +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT + +printf ' fetching %s %s (%s) — once per clone\n' "$tool" "$version" "$platform" >&2 +if ! curl -fsSL --retry 3 --max-time 300 -o "$tmp/$archive" "$base/$archive"; then + printf '\n Could not download %s from %s.\n' "$archive" "$base" >&2 + printf ' It is fetched once and kept in the git directory; re-run when online.\n\n' >&2 + exit 1 +fi + +actual="$(shasum -a 256 "$tmp/$archive" | cut -d' ' -f1)" +if [[ $actual != "$sum" ]]; then + printf '\n %s does not match its pinned checksum — refusing to run it.\n' "$archive" >&2 + printf ' expected %s\n got %s\n\n' "$sum" "$actual" >&2 + exit 1 +fi + +tar -xzf "$tmp/$archive" -C "$tmp" "$tool" +mkdir -p "$store" +# Into the store's own directory first, then renamed: a rename within one +# directory is atomic, so a second run can never find half a binary. +mv "$tmp/$tool" "$store/.$tool.$$" +chmod +x "$store/.$tool.$$" +mv -f "$store/.$tool.$$" "$bin" +printf '%s\n' "$bin" From 92523e13619547cc4cb1d18857dd0f883d5d91e4 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 06:58:45 +0800 Subject: [PATCH 05/12] ci: keep the job token out of .git/config where nothing pushes --- .github/workflows/android.yml | 6 ++++++ .github/workflows/ci.yml | 5 +++++ .github/workflows/ios.yml | 6 ++++++ .github/workflows/release.yml | 9 +++++++++ 4 files changed, 26 insertions(+) diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index 28d9301f6..edc619a4e 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -22,6 +22,9 @@ jobs: steps: - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: + # Nothing here pushes or fetches, so the job token is not left in + # .git/config for the steps after this one. + persist-credentials: false # A push reads its own commit messages below: full history, without # the file contents nothing here looks at. fetch-depth: ${{ github.event_name == 'push' && '0' || '1' }} @@ -63,7 +66,10 @@ jobs: contents: read steps: + # Nothing here pushes, so the job token is not left in .git/config. - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 + with: + persist-credentials: false - name: Setup Java uses: actions/setup-java@b843ebf9b913146b38da2eaa7aeddf76074ae1ad # v6.1.0 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index af79dc67d..76813c3f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,6 +26,11 @@ jobs: - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: fetch-depth: 0 + # Nothing here pushes, and every step after this one runs code the + # branch under test controls: the job token is not left in + # .git/config for it. The `git fetch`es below are anonymous, which a + # public repository allows. + persist-credentials: false # The commit gate first, because it is the only failure that cannot be # fixed by another commit. A message is immutable once pushed, so the diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index 3c3c36ea2..03904da8e 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -34,6 +34,9 @@ jobs: steps: - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: + # Nothing here pushes or fetches, so the job token is not left in + # .git/config for the steps after this one. + persist-credentials: false # A push reads its own commit messages below: full history, without # the file contents nothing here looks at. fetch-depth: ${{ github.event_name == 'push' && '0' || '1' }} @@ -75,7 +78,10 @@ jobs: contents: read steps: + # Nothing here pushes, so the job token is not left in .git/config. - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 + with: + persist-credentials: false # Flutter and Dart come from mise.toml, the same pin CI and every laptop # use. Not subosito/flutter-action with `channel: stable`: that resolves diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ba805a8a4..a8035f85b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -40,6 +40,8 @@ jobs: - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: fetch-depth: 0 + # Nothing in this job pushes; `gh` reads its token from GH_TOKEN. + persist-credentials: false - id: v run: | @@ -159,6 +161,10 @@ jobs: - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: fetch-depth: 0 + # Nothing in this job pushes, and it is the job holding the signing + # keys and the store credentials: the job token is not left in + # .git/config beside them. + persist-credentials: false # Flutter/Dart come from mise.toml, the same pin ci.yml and every # developer use. A published build is the one that most needs to be @@ -433,6 +439,9 @@ jobs: - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: fetch-depth: 0 + # Kept, and the one place it is: `Publish` pushes the snapshot tag + # with `git push`. This job uploads no artifact for it to leak into. + persist-credentials: true - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: From b290b7f71e2b32e7b6f6ab8e7da6019b39c8f6db Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 07:01:36 +0800 Subject: [PATCH 06/12] ci(release): give each job only the token permissions it uses --- .github/workflows/release.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a8035f85b..d07051a67 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,9 +22,19 @@ concurrency: group: release cancel-in-progress: false +# None unless a job asks. The repository's default token can write, and two of +# the three jobs here only read: one of them is the job that holds the signing +# keys and the store credentials, and runs every dependency the build pulls in. +permissions: {} + jobs: version: runs-on: ubuntu-latest + # Read: the checkout, and the release list the ordinal guard compares + # against. A token that can only read is shown published releases and not + # drafts — which is the set the guard means by "published". + permissions: + contents: read outputs: label: ${{ steps.v.outputs.label }} train: ${{ steps.v.outputs.train }} @@ -156,6 +166,10 @@ jobs: - platform: ios runs-on: macos-latest runs-on: ${{ matrix.runs-on }} + # Read, for the checkout. The uploads authenticate with their own store + # credentials, not with this token. + permissions: + contents: read steps: # Full history: tool/release/build_info.sh counts commits and reads tags. - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 @@ -433,6 +447,7 @@ jobs: publish: needs: [version, build] runs-on: ubuntu-latest + # Write: it pushes the snapshot tag and creates the release. permissions: contents: write steps: From b2bcf3703d6e0926ab487fc1eec3d4c47dba7ad9 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 07:07:59 +0800 Subject: [PATCH 07/12] ci(release): pass build values to scripts through the environment --- .github/workflows/release.yml | 57 ++++++++++++------- .../diagnostics/crash_reporting_test.dart | 8 ++- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d07051a67..4fa271186 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -313,26 +313,36 @@ jobs: # the GitHub release, the AAB is the only thing Play accepts. - name: Build env: - # Only the label: gradle takes its versionCode from Flutter's - # `--build-number` above, so there is one source for it rather than - # two that can disagree. + # Only the label under a name the build reads: gradle takes its + # versionCode from Flutter's `--build-number` below, so there is one + # source for it rather than two that can disagree. DPIP_LABEL: ${{ needs.version.outputs.label }} + # The rest under names nothing in the build reads, for the same + # reason. They are in the environment at all so the script expands + # them itself: a `${{ }}` in a `run:` is pasted into its text before + # bash parses it, which leaves every value one quote away from being + # read as code. These come from git and nobody types them, but the + # habit costs nothing, and it is the one every step here follows. + RELEASE_TRAIN: ${{ needs.version.outputs.train }} + RELEASE_CODE: ${{ needs.version.outputs.code }} + RELEASE_DATE: ${{ needs.version.outputs.date }} + RELEASE_PRERELEASE: ${{ needs.version.outputs.prerelease }} run: | # One number for both stores — see tool/release/version.sh for why they can # be aligned. It is also what the release advertises and what the # update check compares, so nothing anywhere has to translate. - COMMON="--build-name=${{ needs.version.outputs.train }} \ - --build-number=${{ needs.version.outputs.code }} \ - --dart-define=DPIP_LABEL=${{ needs.version.outputs.label }} \ - --dart-define=DPIP_CODE=${{ needs.version.outputs.code }} \ - --dart-define=DPIP_TRAIN=${{ needs.version.outputs.train }} \ - --dart-define=DPIP_DATE=${{ needs.version.outputs.date }} \ - --dart-define=DPIP_PRERELEASE=${{ needs.version.outputs.prerelease }}" + COMMON="--build-name=$RELEASE_TRAIN \ + --build-number=$RELEASE_CODE \ + --dart-define=DPIP_LABEL=$DPIP_LABEL \ + --dart-define=DPIP_CODE=$RELEASE_CODE \ + --dart-define=DPIP_TRAIN=$RELEASE_TRAIN \ + --dart-define=DPIP_DATE=$RELEASE_DATE \ + --dart-define=DPIP_PRERELEASE=$RELEASE_PRERELEASE" if [ "${{ matrix.platform }}" = "android" ]; then bash tool/dev/build.sh android $COMMON bash tool/dev/build.sh bundle $COMMON mv build/app/outputs/flutter-apk/app-release.apk \ - "DPIP-${{ needs.version.outputs.label }}.apk" + "DPIP-$DPIP_LABEL.apk" else # Two steps rather than `flutter build ipa`. Flutter passes # `-allowProvisioningUpdates` but never the App Store Connect @@ -359,7 +369,7 @@ jobs: if [ -z "$ipa" ]; then echo "::error::export produced no .ipa"; ls -R "$RUNNER_TEMP/ipa" || true; exit 1 fi - mv "$ipa" "DPIP-${{ needs.version.outputs.label }}.ipa" + mv "$ipa" "DPIP-$DPIP_LABEL.ipa" fi # Resolved at run time, not hardcoded: the directory carries the AGP task @@ -477,15 +487,19 @@ jobs: # a rate-limited release produced a note that credited nobody and said # nothing about why. GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # Through the environment rather than pasted into the script, as in + # `Build`. + RELEASE_LABEL: ${{ needs.version.outputs.label }} + RELEASE_CODE: ${{ needs.version.outputs.code }} run: | if [ "${GITHUB_REF_TYPE}" = "tag" ]; then bash tool/release/notes.sh \ - "${{ needs.version.outputs.label }}" \ - "${{ needs.version.outputs.code }}" --release > NOTES.md + "$RELEASE_LABEL" \ + "$RELEASE_CODE" --release > NOTES.md else bash tool/release/notes.sh \ - "${{ needs.version.outputs.label }}" \ - "${{ needs.version.outputs.code }}" > NOTES.md + "$RELEASE_LABEL" \ + "$RELEASE_CODE" > NOTES.md fi cat NOTES.md @@ -493,6 +507,8 @@ jobs: id: publish env: GH_TOKEN: ${{ github.token }} + RELEASE_LABEL: ${{ needs.version.outputs.label }} + RELEASE_PRERELEASE: ${{ needs.version.outputs.prerelease }} run: | if [ "${GITHUB_REF_TYPE}" = "tag" ]; then tag="${GITHUB_REF_NAME}" @@ -503,14 +519,14 @@ jobs: # An earlier `snapshot/` prefix put the tags somewhere the counter # did not look, so every run computed `a` and this step died on a # tag that already existed. - tag="${{ needs.version.outputs.label }}" + tag="$RELEASE_LABEL" git tag "$tag" git push origin "$tag" fi gh release create "$tag" \ - --title "${{ needs.version.outputs.label }}" \ + --title "$RELEASE_LABEL" \ --notes-file NOTES.md \ - --prerelease="${{ needs.version.outputs.prerelease }}" \ + --prerelease="$RELEASE_PRERELEASE" \ artifacts/* echo "tag=$tag" >> "$GITHUB_OUTPUT" @@ -537,6 +553,7 @@ jobs: # Without it the release lookup runs unauthenticated, at 60 requests # an hour shared across everything on this runner's IP. GITHUB_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ steps.publish.outputs.tag }} run: | if [ -z "${DISCORD_WEBHOOK:-}" ]; then echo "::notice::DISCORD_WEBHOOK is not set — nothing announced" @@ -545,5 +562,5 @@ jobs: # A warning, not a failure. The build is published and the artifacts # are downloadable by the time this runs; turning the job red over an # announcement would say the release did not happen. - python3 tool/release/discord.py "${{ steps.publish.outputs.tag }}" || + python3 tool/release/discord.py "$RELEASE_TAG" || echo "::warning::Discord announcement failed — the release itself is fine" diff --git a/test/core/diagnostics/crash_reporting_test.dart b/test/core/diagnostics/crash_reporting_test.dart index 74f339bdc..2550e6acc 100644 --- a/test/core/diagnostics/crash_reporting_test.dart +++ b/test/core/diagnostics/crash_reporting_test.dart @@ -83,12 +83,18 @@ void main() { final workflow = File( '${Directory.current.path}/.github/workflows/release.yml', ).readAsStringSync(); + // Through the step's environment, not pasted into the script: the + // workflow keeps every `${{ }}` out of `run:` text. expect( workflow, contains( - r'--dart-define=DPIP_PRERELEASE=${{ needs.version.outputs.prerelease }}', + r'RELEASE_PRERELEASE: ${{ needs.version.outputs.prerelease }}', ), ); + expect( + workflow, + contains(r'--dart-define=DPIP_PRERELEASE=$RELEASE_PRERELEASE'), + ); // And that output is false for a tag, which is what production ships. expect( workflow, From d2901118414d89434ec4f50bb36a9d7dcf152f60 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 07:16:10 +0800 Subject: [PATCH 08/12] ci: lint the workflows for security with a pinned zizmor --- .github/workflows/ci.yml | 6 +++++ .github/workflows/release.yml | 22 ++++++++++++++++- AGENTS.md | 5 ++-- pr.md | 2 +- tool/check.sh | 9 +++---- tool/check/workflow_security.sh | 42 +++++++++++++++++++++++++++++++++ tool/internal/pinned_tool.sh | 12 +++++++++- 7 files changed, 89 insertions(+), 9 deletions(-) create mode 100755 tool/check/workflow_security.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 76813c3f4..4504dd88c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -105,6 +105,12 @@ jobs: - name: Workflow gate run: bash tool/check/workflows.sh + # And what they let somebody else do: a moved tag, a token left on disk, + # a job with more permission than it uses, a value pasted into a script. + # The script says more, and pins and checksums the zizmor it runs. + - name: Workflow security gate + run: bash tool/check/workflow_security.sh + # Architecture gate — pure grep, no toolchain, fails fast. - name: Layering gate run: bash tool/check/layering.sh diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4fa271186..8e45ba55a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -180,18 +180,36 @@ jobs: # .git/config beside them. persist-credentials: false + # Four caches are restored below — the SDK, Gradle, pub and Swift + # packages — into the job that signs and ships, and the workflow lint + # (tool/check/workflow_security.sh) flags each one, rightly: whoever can + # write a cache this job restores can put code into a store build. + # + # They are kept, and each is marked as accepted, because of who that is. + # A run on `main` or on a tag restores only what was saved on `main` or + # on that tag; a pull request's caches are scoped to the pull request + # and are never offered here. On `main`, every workflow runs `main`'s own + # code, with one exception: review.yml runs in `main`'s scope on behalf + # of a pull request. Its threat model keeps the pull request's code from + # executing there, but the reviewer it runs is a third party's, and a + # compromised release of it could write a cache. That is the risk being + # accepted, and the pins in review.yml are what bound it; take these + # caches out rather than loosen them. + # # Flutter/Dart come from mise.toml, the same pin ci.yml and every # developer use. A published build is the one that most needs to be # reproducible, so it must not be the one tracking whatever "stable" # happens to mean on the day it runs. - uses: jdx/mise-action@c2a87611a18de5b3828c5652fe268e992400cb5c # v4.3.0 + with: + cache: true # zizmor: ignore[cache-poisoning] — accepted, see above - if: matrix.platform == 'android' uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0 with: distribution: "temurin" java-version: "25" - cache: "gradle" + cache: "gradle" # zizmor: ignore[cache-poisoning] — accepted, see above # Keyed on the lockfile, so it is reused until a dependency actually # moves. Without it every release re-downloads the full package set — @@ -209,6 +227,7 @@ jobs: with: path: ~/.pub-cache key: ${{ runner.os }}-pub-${{ hashFiles('pubspec.lock') }} + lookup-only: false # zizmor: ignore[cache-poisoning] — accepted, see above # The iOS build resolves its dependencies through Swift Package Manager # (there is no Podfile). Uncached, every release re-clones and rebuilds @@ -235,6 +254,7 @@ jobs: key: ${{ runner.os }}-spm-${{ hashFiles('ios/Runner.xcworkspace/xcshareddata/swiftpm/Package.resolved') }} restore-keys: | ${{ runner.os }}-spm- + lookup-only: false # zizmor: ignore[cache-poisoning] — accepted, see above # The committed copy is a stub; the real values come from git. CI also # passes them with --dart-define below, so this is belt and braces — but diff --git a/AGENTS.md b/AGENTS.md index 400c79b8c..491af93dc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -217,6 +217,7 @@ See [commit.md](commit.md). Individually, if you want to fail faster: ```sh tool/check/commits.sh origin/main..HEAD tool/check/workflows.sh +tool/check/workflow_security.sh tool/check/layering.sh tool/check/l10n.sh tool/check/storage.sh @@ -230,8 +231,8 @@ tool/dev/test_ios.sh # only if you touched ios/ — macOS, and slow ``` The bash gates need only bash and python3, so they fail fast without the -toolchain. `workflows.sh` also downloads its linter, at a pinned release with a -pinned checksum, the first time it runs in a clone. `.github/workflows/ci.yml` must stay green; +toolchain. The two workflow gates also download their linters, each at a +pinned release with a pinned checksum, the first time they run in a clone. `.github/workflows/ci.yml` must stay green; `android.yml` / `ios.yml` build artifacts and `review.yml` adds an automated PR review. diff --git a/pr.md b/pr.md index adc66594e..b2953f228 100644 --- a/pr.md +++ b/pr.md @@ -102,7 +102,7 @@ PR 不需要它,例如只改文件的 PR 不會 build App。 | 檢查(`gh pr checks` 顯示的名稱) | 來源 | 什麼時候跑 | 驗什麼 | |---|---|---|---| -| `build` | `ci.yml` | 每個 PR | commit 訊息、分支已 rebase 到 base、workflow/layering/l10n/storage/通知音效/lockfile/tooling 各道 gate、format + analyze、codegen 是最新、完整測試(含覆蓋率上傳) | +| `build` | `ci.yml` | 每個 PR | commit 訊息、分支已 rebase 到 base、workflow 語法/workflow 安全/layering/l10n/storage/通知音效/lockfile/tooling 各道 gate、format + analyze、codegen 是最新、完整測試(含覆蓋率上傳) | | `changes` | `android.yml`、`ios.yml` | 每個 PR | 判斷這個 PR 需不需要 build Android/iOS | | `Android` | `android.yml` | 動到 `android/`、`assets/`、`lib/`、`pubspec.yaml` | Android 能 build | | `iOS` | `ios.yml` | 動到 `ios/`、`assets/`、`lib/`、`pubspec.yaml` | iOS 能 build | diff --git a/tool/check.sh b/tool/check.sh index faf318256..4fac5713e 100755 --- a/tool/check.sh +++ b/tool/check.sh @@ -34,10 +34,11 @@ cached analyze "$(cache_key "${CODE_INPUTS[@]}")" tool/dev/analyze.sh # Most of these finish in under a quarter of a second and are run every time: # hashing their inputs would cost as much as running them, and a cache that -# saves nothing is a cache that can only be wrong. `workflows` downloads its -# linter on the first run in a clone and takes 0.06 s on every one after. -for gate in workflows tooling l10n pubspec_lock notification_sounds build_info \ - widget_township_resources; do +# saves nothing is a cache that can only be wrong. The two workflow gates +# download their linter on the first run in a clone, and take a few hundredths +# of a second on every run after that. +for gate in workflows workflow_security tooling l10n pubspec_lock \ + notification_sounds build_info widget_township_resources; do step "check/$gate" "tool/check/$gate.sh" done diff --git a/tool/check/workflow_security.sh b/tool/check/workflow_security.sh new file mode 100755 index 000000000..693002a6b --- /dev/null +++ b/tool/check/workflow_security.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# What a workflow lets somebody else do, not merely whether it parses. +# +# tool/check/workflow_security.sh +# +# zizmor, at the release tool/internal/pinned_tool.sh pins and verifies, over +# everything under .github/ — the workflows and dependabot.yml. It is what keeps +# them the way they are now: every action pinned to a commit, no job token left +# on disk where nothing pushes, no job holding more permission than it uses, no +# value pasted into a script, and no secret-holding trigger that runs a pull +# request's code. Each of those is a one-line regression nobody would notice in +# review, and the workflows hold the store signing keys. +# +# Every finding at zizmor's default persona and severity fails. A finding that +# is accepted on purpose is marked on its own line — `# zizmor: ignore[]` +# with the reason beside it — so the exception is read by whoever edits that +# line, rather than kept in a config file nobody opens. To list them: +# +# grep -rn 'zizmor: ignore' .github +# +# `--offline`, here and in CI alike: the audits that ask GitHub (whether a +# pinned commit really belongs to the action's repository, which releases are +# known to be vulnerable) need a token and can answer differently tomorrow +# about a file that has not changed. A gate has to give one answer per file. +set -euo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")/../.." + +zizmor="$(tool/internal/pinned_tool.sh zizmor)" + +# `--strict-collection` makes a file zizmor cannot parse a failure; by default +# it is a warning, and the file is simply not audited. +if ! "$zizmor" --offline --strict-collection --quiet .github; then + cat >&2 <<'EOF' + +Each finding above names its audit, with a link to what it means and how to +fix it. Fix it; or, when it is accepted on purpose, mark the flagged line +`# zizmor: ignore[]` and say why beside it. +EOF + exit 1 +fi + +echo "workflow security OK — zizmor found nothing." diff --git a/tool/internal/pinned_tool.sh b/tool/internal/pinned_tool.sh index 73cd60539..534fadd6a 100755 --- a/tool/internal/pinned_tool.sh +++ b/tool/internal/pinned_tool.sh @@ -2,6 +2,7 @@ # A third-party binary at a pinned release, verified before anything runs it. # # tool/internal/pinned_tool.sh actionlint # prints the binary's path +# tool/internal/pinned_tool.sh zizmor # # For tools that are not the app's toolchain and so do not belong in # mise.toml: mise-action installs everything that file lists in every workflow @@ -28,6 +29,7 @@ source "$(dirname "${BASH_SOURCE[0]}")/../dev/_lib.sh" cd "$(repo_root)" readonly ACTIONLINT=1.7.12 +readonly ZIZMOR=1.30.1 tool="${1:-}" case "$tool" in @@ -35,8 +37,12 @@ actionlint) version=$ACTIONLINT base="https://github.com/rhysd/actionlint/releases/download/v$ACTIONLINT" ;; +zizmor) + version=$ZIZMOR + base="https://github.com/zizmorcore/zizmor/releases/download/v$ZIZMOR" + ;; *) - printf 'usage: tool/internal/pinned_tool.sh actionlint\n' >&2 + printf 'usage: tool/internal/pinned_tool.sh \n' >&2 exit 2 ;; esac @@ -55,6 +61,10 @@ actionlint/darwin-arm64) pin="actionlint_${ACTIONLINT}_darwin_arm64.tar.gz aba9c actionlint/darwin-x64) pin="actionlint_${ACTIONLINT}_darwin_amd64.tar.gz 5b44c3bc2255115c9b69e30efc0fecdf498fdb63c5d58e17084fd5f16324c644" ;; actionlint/linux-x64) pin="actionlint_${ACTIONLINT}_linux_amd64.tar.gz 8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8" ;; actionlint/linux-arm64) pin="actionlint_${ACTIONLINT}_linux_arm64.tar.gz 325e971b6ba9bfa504672e29be93c24981eeb1c07576d730e9f7c8805afff0c6" ;; +zizmor/darwin-arm64) pin="zizmor-aarch64-apple-darwin.tar.gz e28d22b087f9ebb8d99da6e740d348c930f559961c7c3f12badda54f882195a2" ;; +zizmor/darwin-x64) pin="zizmor-x86_64-apple-darwin.tar.gz 10e6b18b11ea07e515a16f0f0518c7b07527bc9977c1fd5698181ce7f3554202" ;; +zizmor/linux-x64) pin="zizmor-x86_64-unknown-linux-gnu.tar.gz e65324f4430c2717591937edcec90ccbefaf14c174f8ec9415e03ca875b46e1a" ;; +zizmor/linux-arm64) pin="zizmor-aarch64-unknown-linux-gnu.tar.gz 7ff1dce33bdd18fd2a4affe63bdd47efcccca97b2cec1c1863ec26e9e2647540" ;; *) # A failure, not a skip. The gates that call this are in tool/check.sh, and # a gate that quietly does not run turns "green here means green in CI" into From 71a0eff4133246ebc69c797b9c494332a2280ba6 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 07:27:02 +0800 Subject: [PATCH 09/12] ci: require package imports, which the layering gate relies on --- analysis_options.yaml | 5 +++++ third_party/meshtastic_flutter/analysis_options.yaml | 12 ++++++++++++ tool/check/layering.sh | 4 ++++ 3 files changed, 21 insertions(+) create mode 100644 third_party/meshtastic_flutter/analysis_options.yaml diff --git a/analysis_options.yaml b/analysis_options.yaml index 345c5b0ea..da0447903 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -29,3 +29,8 @@ analyzer: linter: rules: avoid_print: true + # Every import in lib/ is `package:dpip/…`. tool/check/layering.sh reads + # imports with grep and recognises only that form, so a relative import + # would carry a cross-layer dependency straight past it — this is what + # makes "the gate saw every import" true rather than merely usual. + always_use_package_imports: true diff --git a/third_party/meshtastic_flutter/analysis_options.yaml b/third_party/meshtastic_flutter/analysis_options.yaml new file mode 100644 index 000000000..b55515539 --- /dev/null +++ b/third_party/meshtastic_flutter/analysis_options.yaml @@ -0,0 +1,12 @@ +# The app's rules, minus the one that is about the app. +# +# `always_use_package_imports` exists for tool/check/layering.sh, which reads +# dpip's own imports and only sees `package:dpip/…`. This package is a vendored +# fork whose protobuf sources are generated with relative imports; rewriting +# them would be undone by the next regeneration and would buy the layering gate +# nothing, since it never looks here. +include: ../../analysis_options.yaml + +linter: + rules: + always_use_package_imports: false diff --git a/tool/check/layering.sh b/tool/check/layering.sh index b86679f1a..1d374e832 100755 --- a/tool/check/layering.sh +++ b/tool/check/layering.sh @@ -35,6 +35,10 @@ while IFS= read -r file; do ;; esac + # Only `package:dpip/…` imports are read. That is every first-party import + # in lib/ because the analyzer insists on it (`always_use_package_imports` + # in analysis_options.yaml): a relative import would otherwise walk a + # cross-layer dependency straight past this gate without a word. imports=$(grep -oE "import 'package:dpip/[^']+'" "$file" \ | sed -E "s|import 'package:dpip/([^']+)'|\1|") || true [ -z "$imports" ] && continue From acd378bd17ca558898952c42cf07f45b9154cf0c Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 07:34:44 +0800 Subject: [PATCH 10/12] docs(notify): describe push as awesome_notifications_fcm carries it --- ARCHITECTURE.md | 40 +++++++---- ios/Runner/AppDelegate.swift | 6 +- .../notifications/notification_service.dart | 66 ++++++++----------- 3 files changed, 58 insertions(+), 54 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index ab5d9e7d7..694c3ae05 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -251,15 +251,26 @@ blocks: `LoadingView` / `ErrorView` / `EmptyView`. Template consumer: ## Push notifications -(`core/notifications/`): `firebase_messaging` owns the -FCM/APNs transport (token + message receipt) on both platforms; `awesome_ -notifications` renders the rich per-channel notification and routes taps. -(Legacy used `awesome_notifications_fcm`, but that plugin's iOS pod breaks the -rewrite's newer Flutter/scene build — one FCM plugin is simpler and builds -clean.) Keep Firebase pinned (a newer major raises the min iOS target). Backend -sends **data** messages (`channel`/`id`/`title`/`body`); foreground + data-only -background messages display via awesome so each honours its channel; a -`notification`-payload message is shown by the OS. Alert categories live in +(`core/notifications/`): `awesome_notifications_fcm` owns the +remote-push transport (token + message receipt) on both platforms; +`awesome_notifications` renders the rich per-channel notification and routes +taps. `firebase_messaging` is not a dependency — upstream says the two must +not coexist, and its jobs are covered: awesome_notifications_fcm's token +handlers replace its refresh stream, and on iOS the app's own +notification-centre delegate decides foreground presentation. Keep Firebase +pinned (`firebase_core`; a newer major raises the min iOS target). The push +token is the **raw APNs device token** on iOS, read from the app delegate +(`ApnsTokenPlugin`), and the **FCM registration token** on Android; +`NotificationService` stores only the platform's own kind, because the +backend keys on it and registering the other one fails silently. Backend +sends **data** messages (`channel`/`id`/`title`/`body`, or the same fields +packed into a JSON `content` string); `onFcmSilentData` displays them via +awesome so each honours its channel. A `notification`-payload message in the +background is shown by the OS — on Android against the plain channel ids +`PlainChannels` mirrors, so it still gets its channel's sound. On iOS, a push +published straight to APNs is one awesome cannot present in the foreground; +`NotificationDelegateProxy` in `AppDelegate.swift` presents it and forwards +everything else to awesome. Alert categories live in `notification_channels.dart` — bump `version` when one changes so Android re-creates it. Permission requests go through `NotificationService`, which asks the plugin to prompt **only** when the OS @@ -275,10 +286,13 @@ the app with no channels *and* no push transport. (`initialize` needs at least one channel, so the fallback seeds itself with the first one that is accepted.) `tool/check/notification_sounds.sh` catches an unresolvable `resource://raw/` before it ships. Taps funnel through `NotificationTaps` (core carries the channel -key; `app/` maps it to an `AppRoutes` tab). **External, not code:** upload an -APNs auth key to the Firebase console for iOS; push only works on a physical -device; permission is requested after the first frame for now (move to -onboarding). The push token registers through the location feature +key; `app/` maps it to an `AppRoutes` tab). Notification permission is asked +for from onboarding and from 更多 → 權限檢查 (`PermissionChecklist`), never at +launch. **External, not code:** push only works on a physical device; on +Android, FCM registration needs the app's signing SHA-1 in the Firebase +console; the backend publishes iOS pushes straight to APNs, so the APNs auth +key in the Firebase console matters only to a message sent through FCM. The +push token registers through the location feature (`/v2/location`, `DeviceLocationReporter` in `core/geo/`) on meaningful moves, and `DeviceRegistration` reports once more whenever the stored token changes and once a day without a successful report — a device that never moves would diff --git a/ios/Runner/AppDelegate.swift b/ios/Runner/AppDelegate.swift index b8cf5602a..3b6a9b6c1 100644 --- a/ios/Runner/AppDelegate.swift +++ b/ios/Runner/AppDelegate.swift @@ -34,8 +34,10 @@ import UserNotifications override func applicationDidBecomeActive(_ application: UIApplication) { super.applicationDidBecomeActive(application) - // Re-assert: firebase_messaging's proxy can claim the delegate again from - // its own launch observer, and whoever is last wins. + // Re-assert: awesome_notifications makes itself the notification-centre + // delegate every time the plugin is attached to an engine — including the + // background engine awesome_notifications_fcm starts for a silent push — + // and whoever is last wins. NotificationDelegateProxy.shared.install() } diff --git a/lib/core/notifications/notification_service.dart b/lib/core/notifications/notification_service.dart index d3b72774b..52f706e99 100644 --- a/lib/core/notifications/notification_service.dart +++ b/lib/core/notifications/notification_service.dart @@ -25,17 +25,19 @@ const String _fallbackChannelKey = 'announcement-general-v2'; /// Sets up push notifications. /// -/// `firebase_messaging` owns the FCM/APNs transport (token + message receipt) on -/// both platforms, and `awesome_notifications` renders the rich per-channel -/// notification and routes taps. (The legacy app used `awesome_notifications_fcm` -/// on Android, but that plugin's iOS pod is incompatible with the rewrite's -/// newer Flutter/scene lifecycle — one FCM plugin is simpler and builds cleanly.) +/// `awesome_notifications_fcm` owns the remote-push transport (token + message +/// receipt) on both platforms, and `awesome_notifications` renders the rich +/// per-channel notification and routes taps. `firebase_messaging` is not a +/// dependency — see [_initMessaging]. /// /// Message flow: the backend sends **data** messages (`channel`/`id`/`title`/ -/// `body`). Foreground messages and background data-only messages are displayed -/// via awesome so every notification honours its channel; taps funnel through -/// [NotificationTaps]. A `notification`-payload message is displayed by the OS -/// directly (its tap arrives via `onMessageOpenedApp`). +/// `body`, or the same fields packed into a JSON `content` string — see +/// [contentFromData]). [onFcmSilentData] displays them via awesome so every +/// notification honours its channel; taps funnel through [NotificationTaps]. +/// A `notification`-payload message in the background is displayed by the OS +/// directly, and on iOS the foreground presentation of a push sent straight to +/// APNs is decided by the app's own notification-centre delegate +/// (`NotificationDelegateProxy` in ios/Runner/AppDelegate.swift). class NotificationService { NotificationService( this._settings, { @@ -524,38 +526,17 @@ class NotificationService { }); } - /// Fetches the push token and persists it as [SettingKeys.pushToken] — - /// the identifier every backend registration call (`/v2/location`, - /// `/v2/notify`) keys on. - /// - /// **Which token, per platform, matters**: on iOS that has to be the raw - /// **APNs** device token, not Firebase's FCM registration token — a - /// different string the backend's own registration endpoints don't - /// recognise (confirmed live: registering with the FCM token 202'd, as a - /// write with no format check, but every subsequent `/v2/notify` lookup by - /// that same value still 401'd — the legacy app stored exactly and only the - /// APNs token on iOS for this reason, in `fcm.dart`). Android's backend - /// registration keys on the FCM token, which is `getToken()`'s value there. - /// - /// `getToken()` is still awaited on iOS (its FCM-pipeline side effects are - /// unrelated to which value gets persisted); only what gets *stored* differs - /// by platform. The APNs token needs `registerForRemoteNotifications` to have - /// completed first (handled by the FlutterFire app-delegate proxy) and - /// arrives asynchronously, so poll briefly before giving up — else `getToken` - /// itself throws `apns-token-not-set`. Recent iOS 16+ Simulators do provision - /// an APNs token (older ones return null); either way delivery still needs - /// the APNs auth key uploaded to the Firebase console. Best-effort: a failure - /// just leaves the token unset until [requestPermission] or - /// `onTokenRefresh` tries again. - /// The one place [SettingKeys.pushToken] is written. + /// The one place [SettingKeys.pushToken] is written — the identifier every + /// backend registration call (`/v2/location`, `/v2/notify`) keys on. /// /// Tokens arrive from three directions — awesome_notifications_fcm's two - /// handlers, firebase's refresh stream, and the launch-time fetch — and the - /// two kinds are **not interchangeable**: the backend keys on the raw APNs - /// token on iOS and the FCM registration token on Android. Registering the - /// wrong one is not a loud failure; it was measured to 202 on the write and - /// then 401 on every later lookup, which reads as "push is broken" with no - /// clue why. + /// handlers and the launch-time [_fetchToken] — and the two kinds are **not + /// interchangeable**: the backend keys on the raw APNs token on iOS and the + /// FCM registration token on Android. Registering the wrong one is not a + /// loud failure; it was measured to 202 on the write and then 401 on every + /// later lookup, which reads as "push is broken" with no clue why. (The + /// legacy app stored only the APNs token on iOS for this reason, in + /// `fcm.dart`.) /// /// So every writer states which kind it holds and this decides, instead of /// each caller repeating a platform test — the version that did not repeat @@ -575,6 +556,13 @@ class NotificationService { /// The APNs device token, read from the iOS side that receives it. static const _apns = MethodChannel('com.exptech.dpip/apns_token'); + /// Fetches this platform's push token and hands it to [_storeToken]. + /// + /// On iOS that is the raw APNs device token, read from the app delegate + /// (`ApnsTokenPlugin`) and polled for a few seconds, because iOS hands it + /// over a moment after launch. On Android it is the FCM registration token. + /// Best-effort: a failure leaves the token unset until [requestPermission] + /// or one of awesome_notifications_fcm's token handlers supplies it. Future _fetchToken() async { try { if (Platform.isIOS) { From b486504e77ac6a6c73ec6e35c400bbf068ca1753 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 07:41:52 +0800 Subject: [PATCH 11/12] docs(location): drop the upload lottery the reporter never had --- lib/core/geo/device_location_reporter.dart | 13 ++++++------- test/core/geo/device_location_reporter_test.dart | 2 -- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/core/geo/device_location_reporter.dart b/lib/core/geo/device_location_reporter.dart index 2ba1a5174..45f72d846 100644 --- a/lib/core/geo/device_location_reporter.dart +++ b/lib/core/geo/device_location_reporter.dart @@ -15,13 +15,12 @@ import 'package:dpip/core/settings/settings_store.dart'; /// `LocationApi.updateDeviceLocation` POST). Consecutive identical fixes are /// skipped, and a report failure is logged, never thrown. /// -/// Upload gates (in order): -/// 1. **60s throttle** — keyed by [SettingKeys.deviceLocationUpdatedAtMs]. -/// Inside the window: still publish [fixes], skip API silently. -/// 2. **1-in-4 lottery** — after the throttle clears; a miss skips without -/// burning the stamp so a later move can try again. -/// 3. On a fired attempt (or throw), the stamp is written — so a missing push -/// token (`onMoved` → `false`) does not burn the window. +/// Uploads are throttled to one per [minUploadInterval] (60 s by default), +/// keyed by [SettingKeys.deviceLocationUpdatedAtMs]. Inside the window the fix +/// is still published on [fixes] and the API is skipped silently; outside it, +/// every move is reported. The stamp is written only when an upload was +/// attempted (or threw), so a missing push token (`onMoved` → `false`) does not +/// burn the window. /// /// The positions come from a **factory** (`Stream Function()`), not a /// single stream instance, because geolocator's position stream *errors and diff --git a/test/core/geo/device_location_reporter_test.dart b/test/core/geo/device_location_reporter_test.dart index cead10f0d..4b41c1ce7 100644 --- a/test/core/geo/device_location_reporter_test.dart +++ b/test/core/geo/device_location_reporter_test.dart @@ -16,8 +16,6 @@ void main() { return SettingsStore.inMemory(initial); } - // Deterministic win — production defaults to 1-in-4. - test('reports each distance-triggered move; skips identical fixes', () async { final positions = StreamController(); final reported = []; From 2546891fd736354c3094b1ff2f27aec8b7887e61 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Fri, 16 Oct 2026 07:52:45 +0800 Subject: [PATCH 12/12] ci: make every device-clock read in lib/ say why it is right --- .github/workflows/ci.yml | 5 + AGENTS.md | 1 + ARCHITECTURE.md | 5 + lib/core/diagnostics/diagnostics_report.dart | 7 + lib/core/geo/device_location_reporter.dart | 3 + lib/core/geo/location_service.dart | 2 + lib/core/logging/log.dart | 2 + lib/core/logging/log_store.dart | 4 + lib/core/network/endpoint_health.dart | 2 + lib/core/network/etag_cache_store.dart | 2 + lib/core/network/network_usage_store.dart | 2 + lib/core/realtime/app_time.dart | 1 + lib/core/realtime/clock.dart | 2 + lib/core/realtime/http_time_source.dart | 3 + .../earthquake/data/monitor_demo.dart | 8 + .../status/data/cloudflare_status_api.dart | 2 + .../status/data/server_status_api.dart | 2 + pr.md | 2 +- test/tool/wall_clock_gate_test.dart | 169 ++++++++++++++++++ tool/check.sh | 8 +- tool/check/wall_clock.sh | 148 +++++++++++++++ 21 files changed, 375 insertions(+), 5 deletions(-) create mode 100644 test/tool/wall_clock_gate_test.dart create mode 100755 tool/check/wall_clock.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4504dd88c..fcb4614c9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -125,6 +125,11 @@ jobs: - name: Storage gate run: bash tool/check/storage.sh + # Wall-clock gate — every DateTime.now() in lib/ says why the device + # clock is the right one. See ARCHITECTURE.md → Calibrated time. + - name: Wall-clock gate + run: bash tool/check/wall_clock.sh + # Every channel sound must resolve, or the channel is rejected on device. - name: Notification-sound gate run: bash tool/check/notification_sounds.sh diff --git a/AGENTS.md b/AGENTS.md index 491af93dc..805481f32 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -221,6 +221,7 @@ tool/check/workflow_security.sh tool/check/layering.sh tool/check/l10n.sh tool/check/storage.sh +tool/check/wall_clock.sh tool/check/pubspec_lock.sh tool/check/notification_sounds.sh tool/check/tooling.sh diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 694c3ae05..71b96ba8c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -236,6 +236,11 @@ necessary. resume). Read corrected time anywhere via the global `AppTime` facade (`AppTime.utc` / `AppTime.utc8`, the fixed-offset Taipei wall clock) — never `DateTime.now()` for anything the server timestamps. +`tool/check/wall_clock.sh` holds every device-clock read in `lib/` to that: +each one carries `// wall-clock: `, on its line or in the comment +directly above, saying why device time is right there — a store that only +compares its own stamps, an OS stamp aged by the clock that made it, the clock +being calibrated. ## Async-state UI diff --git a/lib/core/diagnostics/diagnostics_report.dart b/lib/core/diagnostics/diagnostics_report.dart index 482b0d700..f7adc13a6 100644 --- a/lib/core/diagnostics/diagnostics_report.dart +++ b/lib/core/diagnostics/diagnostics_report.dart @@ -105,6 +105,8 @@ String _lastAttempt(Map d) { final at = d['lastAttemptAt'] ?? d['lastReportAt']; if (at is! int) return 'never'; final when = DateTime.fromMillisecondsSinceEpoch(at); + // wall-clock: the native background task stamps this with the device + // clock, so its age is measured by the same one. Likewise below. final age = DateTime.now().difference(when); final ok = (d['lastAttemptOk'] ?? d['lastReportOk']) == true; final code = d['lastAttemptCode'] ?? d['lastReportCode']; @@ -117,6 +119,7 @@ String _lastSuccess(Map d) { final at = d['lastSuccessAt'] ?? fallback; if (at is! int) return 'never'; final code = d['lastSuccessCode'] ?? d['lastReportCode']; + // wall-clock: a native-task stamp, in device time. return '${_age(DateTime.now().difference(DateTime.fromMillisecondsSinceEpoch(at)))} ' 'ago${code is int ? ' · $code' : ''}'; } @@ -127,6 +130,7 @@ String _throttled(Map d) { if (count is! int || count == 0) return 'none'; final at = d['lastThrottledAt']; if (at is! int) return '$count'; + // wall-clock: a native-task stamp, in device time. final age = DateTime.now().difference( DateTime.fromMillisecondsSinceEpoch(at), ); @@ -137,6 +141,7 @@ String _nextAlarm(Map d) { final at = d['nextAlarmAt']; if (at is! int) return '—'; final remaining = DateTime.fromMillisecondsSinceEpoch(at) + // wall-clock: the OS alarm scheduler's own time, in device time. .difference(DateTime.now()); if (remaining.isNegative) return 'due (OS may deliver it late)'; return 'in ${_age(remaining)}'; @@ -145,6 +150,7 @@ String _nextAlarm(Map d) { String _relativeTime(Object? value, {required String absent}) { if (value is! int) return absent; final delta = DateTime.fromMillisecondsSinceEpoch(value) + // wall-clock: every value passed here is a native stamp, device time. .difference(DateTime.now()); return delta.isNegative ? '${_age(-delta)} ago' : 'in ${_age(delta)}'; } @@ -176,6 +182,7 @@ String _watchdogRepairs(Map d) { String _lastGeofenceTransition(Map d) { final at = d['lastGeofenceTransitionAt']; if (at is! int) return 'never'; + // wall-clock: a native-task stamp, in device time. final age = DateTime.now().difference( DateTime.fromMillisecondsSinceEpoch(at), ); diff --git a/lib/core/geo/device_location_reporter.dart b/lib/core/geo/device_location_reporter.dart index 45f72d846..4e3d8917b 100644 --- a/lib/core/geo/device_location_reporter.dart +++ b/lib/core/geo/device_location_reporter.dart @@ -44,6 +44,9 @@ class DeviceLocationReporter { this._onReported, this.minUploadInterval = const Duration(seconds: 60), DateTime Function()? now, + // wall-clock: the throttle stamp is written and compared by this class + // alone, so it only has to agree with itself; a stamp a clock change left + // in the future is handled in [_handle]. }) : _now = now ?? DateTime.now; final Stream Function() _positions; diff --git a/lib/core/geo/location_service.dart b/lib/core/geo/location_service.dart index 27fe5f294..f291c4ed2 100644 --- a/lib/core/geo/location_service.dart +++ b/lib/core/geo/location_service.dart @@ -371,6 +371,8 @@ class LocationService { /// any age passes. Same convention as `staleness.dart`. static bool _isFresh(DateTime timestamp) { final calibrated = AppTime.utc.difference(AppTime.fromDevice(timestamp)); + // wall-clock: the fix's age by the clock that stamped it, kept beside the + // calibrated age on purpose (see above). final byDevice = DateTime.now().toUtc().difference(timestamp.toUtc()); if (calibrated.isNegative || byDevice.isNegative) return false; final age = calibrated > byDevice ? calibrated : byDevice; diff --git a/lib/core/logging/log.dart b/lib/core/logging/log.dart index d8ed59880..acd3ba6ef 100644 --- a/lib/core/logging/log.dart +++ b/lib/core/logging/log.dart @@ -230,6 +230,8 @@ abstract final class Log { /// its own 24-hour retention (see [LogStore]); this only trims what Talker /// holds for the current session. static void pruneOlderThan(Duration age) { + // wall-clock: Talker stamps its history with the device clock, so the + // cutoff has to come from the same one. final cutoff = DateTime.now().subtract(age); talker.history.removeWhere((entry) => entry.time.isBefore(cutoff)); } diff --git a/lib/core/logging/log_store.dart b/lib/core/logging/log_store.dart index 2194aae46..23ad78997 100644 --- a/lib/core/logging/log_store.dart +++ b/lib/core/logging/log_store.dart @@ -74,6 +74,10 @@ class LogStore { DateTime Function()? now, Duration flushInterval = const Duration(seconds: 3), int flushAt = 64, + // wall-clock: every line is stamped in device time — by Talker, or by the + // native side when it hands its lines over — so retention measures against + // the same clock. The row cap is what keeps a clock change from emptying + // the table. }) : _now = now ?? DateTime.now, _flushInterval = flushInterval, _flushAt = flushAt; diff --git a/lib/core/network/endpoint_health.dart b/lib/core/network/endpoint_health.dart index 6e6c6fee2..17d340286 100644 --- a/lib/core/network/endpoint_health.dart +++ b/lib/core/network/endpoint_health.dart @@ -232,6 +232,8 @@ class EndpointHealthMonitor extends ChangeNotifier { ); } + // wall-clock: when this device saw its own request succeed or fail; never + // compared with anything a server stamped. static DateTime _now() => DateTime.now(); static String _hostOf(String url) { diff --git a/lib/core/network/etag_cache_store.dart b/lib/core/network/etag_cache_store.dart index 6763d19e0..25f58a6de 100644 --- a/lib/core/network/etag_cache_store.dart +++ b/lib/core/network/etag_cache_store.dart @@ -725,6 +725,8 @@ class EtagCacheStore { /// just fetched and keep what nobody has touched in weeks. Monotonicity is /// the only property this key actually needs. int _lruStamp() { + // wall-clock: a sequence key, only ever compared with other rows' keys — + // see above. final now = DateTime.now().millisecondsSinceEpoch; if (now > _lastStamp) _lastStamp = now; return _lastStamp; diff --git a/lib/core/network/network_usage_store.dart b/lib/core/network/network_usage_store.dart index b4ab12359..a80596e91 100644 --- a/lib/core/network/network_usage_store.dart +++ b/lib/core/network/network_usage_store.dart @@ -99,6 +99,8 @@ class NetworkUsageStore { DateTime Function()? now, this.flushInterval = const Duration(seconds: 2), this.flushEvery = 64, + // wall-clock: buckets are keyed and swept by this one clock, and nothing + // in them was stamped by a server. }) : _now = now ?? DateTime.now; final SqliteDatabase _db; diff --git a/lib/core/realtime/app_time.dart b/lib/core/realtime/app_time.dart index e4b2181f0..0daadd453 100644 --- a/lib/core/realtime/app_time.dart +++ b/lib/core/realtime/app_time.dart @@ -29,6 +29,7 @@ abstract final class AppTime { } /// Calibrated current time in UTC (device time until the first sync). + // wall-clock: the fallback before [install] — nothing calibrated exists yet. static DateTime get utc => _clock?.now() ?? DateTime.now().toUtc(); /// Calibrated current time at Taiwan's fixed +8 offset. diff --git a/lib/core/realtime/clock.dart b/lib/core/realtime/clock.dart index 284c97759..24854f95f 100644 --- a/lib/core/realtime/clock.dart +++ b/lib/core/realtime/clock.dart @@ -14,6 +14,8 @@ abstract interface class Clock { class SystemClock implements Clock { const SystemClock(); + // wall-clock: this is the device clock, by name — what [ServerClock] + // corrects, and what a caller asks for when it means device time. @override DateTime now() => DateTime.now(); } diff --git a/lib/core/realtime/http_time_source.dart b/lib/core/realtime/http_time_source.dart index e50dbac71..3b5ec830a 100644 --- a/lib/core/realtime/http_time_source.dart +++ b/lib/core/realtime/http_time_source.dart @@ -179,6 +179,8 @@ class HttpTimeSource implements ServerTimeSource { request.headers.set(HttpHeaders.cacheControlHeader, 'no-store'); request.followRedirects = false; + // wall-clock: the device clock is what this measures against the + // server; the offset between the two is the result. Likewise below. final sentAtMs = DateTime.now().toUtc().millisecondsSinceEpoch; final response = await request.close(); @@ -197,6 +199,7 @@ class HttpTimeSource implements ServerTimeSource { } return buffer + chunk; }); + // wall-clock: as for sentAtMs. final receivedAtMs = DateTime.now().toUtc().millisecondsSinceEpoch; // Unix milliseconds with a fractional part ("1791454316238.287"); the diff --git a/lib/features/earthquake/data/monitor_demo.dart b/lib/features/earthquake/data/monitor_demo.dart index f77aa3267..33712f802 100644 --- a/lib/features/earthquake/data/monitor_demo.dart +++ b/lib/features/earthquake/data/monitor_demo.dart @@ -57,6 +57,7 @@ abstract final class MonitorDemo { _depth = _severeDepth; _magnitude = _severeMagnitude; _loaded = true; + // wall-clock: debug-only, and the wrong clock — see [origin]. _origin = DateTime.now().toUtc(); Log.debug( 'monitor demo → severe preset: mag=$_magnitude depth=$_depth ' @@ -76,6 +77,7 @@ abstract final class MonitorDemo { _depth = report.depth; _magnitude = report.magnitude; _loaded = true; + // wall-clock: debug-only, and the wrong clock — see [origin]. _origin = DateTime.now().toUtc(); Log.debug( 'monitor demo → ${report.id}: mag=$_magnitude depth=$_depth ' @@ -108,6 +110,10 @@ abstract final class MonitorDemo { /// When the demo event "started" — the moment the app first built an alert. /// Re-anchored when [load] pulls the real report, so the synthetic wavefront /// looks freshly started rather than a few hundred milliseconds stale. + // wall-clock: debug-only, and the wrong clock, left as it is for now. This + // stands in for a server stamp, and the EEW card ages it against + // AppTime.utc, so on a phone whose clock is off the demo countdown is off by + // as much. It should read AppTime.utc, as should the two demo sources below. static DateTime get origin => _origin ??= DateTime.now().toUtc(); static DateTime? _origin; @@ -129,6 +135,7 @@ class StartupEewDemoSource extends RealtimeSource> { status: 0, isFinal: false, info: EewInfo( + // wall-clock: debug-only, the wrong clock — see MonitorDemo.origin. time: (clock ?? DateTime.now)().toUtc().millisecondsSinceEpoch, longitude: 121.7, latitude: 23.9, @@ -243,6 +250,7 @@ class DemoEewSource extends RealtimeSource> { /// boxes on the monitor, exactly as a real frame's alerting stations do. class DemoRtsSource extends RealtimeSource { DemoRtsSource({required this.stations, DateTime Function()? clock}) + // wall-clock: debug-only, the wrong clock — see MonitorDemo.origin. : _clock = clock ?? DateTime.now { _tick = Timer.periodic(const Duration(seconds: 1), (_) => _update()); _init(); diff --git a/lib/features/status/data/cloudflare_status_api.dart b/lib/features/status/data/cloudflare_status_api.dart index 3c8d4f098..57187f330 100644 --- a/lib/features/status/data/cloudflare_status_api.dart +++ b/lib/features/status/data/cloudflare_status_api.dart @@ -64,5 +64,7 @@ CloudflareStatus parseCloudflareStatus(Object? body, {DateTime? at}) { return a.name.compareTo(b.name); }); + // wall-clock: when this device ran the query, shown against the device's + // own clock; nothing compares it with a server stamp. return CloudflareStatus(components: kept, recordedAt: at ?? DateTime.now()); } diff --git a/lib/features/status/data/server_status_api.dart b/lib/features/status/data/server_status_api.dart index 9733c4b95..f603c19c9 100644 --- a/lib/features/status/data/server_status_api.dart +++ b/lib/features/status/data/server_status_api.dart @@ -97,6 +97,8 @@ ServerStatus parseStatus(Object? body, {DateTime? at}) { } return ServerStatus( + // wall-clock: when this device ran the query, shown against the device's + // own clock; nothing compares it with a server stamp. recordedAt: at ?? DateTime.now(), down: StatusMetric(value: scalar('status')), errorRate: StatusMetric( diff --git a/pr.md b/pr.md index b2953f228..a5b635009 100644 --- a/pr.md +++ b/pr.md @@ -102,7 +102,7 @@ PR 不需要它,例如只改文件的 PR 不會 build App。 | 檢查(`gh pr checks` 顯示的名稱) | 來源 | 什麼時候跑 | 驗什麼 | |---|---|---|---| -| `build` | `ci.yml` | 每個 PR | commit 訊息、分支已 rebase 到 base、workflow 語法/workflow 安全/layering/l10n/storage/通知音效/lockfile/tooling 各道 gate、format + analyze、codegen 是最新、完整測試(含覆蓋率上傳) | +| `build` | `ci.yml` | 每個 PR | commit 訊息、分支已 rebase 到 base、workflow 語法/workflow 安全/layering/l10n/storage/wall-clock/通知音效/lockfile/tooling 各道 gate、format + analyze、codegen 是最新、完整測試(含覆蓋率上傳) | | `changes` | `android.yml`、`ios.yml` | 每個 PR | 判斷這個 PR 需不需要 build Android/iOS | | `Android` | `android.yml` | 動到 `android/`、`assets/`、`lib/`、`pubspec.yaml` | Android 能 build | | `iOS` | `ios.yml` | 動到 `ios/`、`assets/`、`lib/`、`pubspec.yaml` | iOS 能 build | diff --git a/test/tool/wall_clock_gate_test.dart b/test/tool/wall_clock_gate_test.dart new file mode 100644 index 000000000..b52b8e5be --- /dev/null +++ b/test/tool/wall_clock_gate_test.dart @@ -0,0 +1,169 @@ +/// tool/check/wall_clock.sh, the gate that makes every device-clock read in +/// lib/ say why the device clock is the right one. +/// +/// A gate like this has two ways to fail and they are not equally bad. Flagging +/// a line it should not costs a comment; missing a line it should flag is the +/// silent failure the gate exists to end — a `DateTime.now()` subtracted from a +/// server stamp, on the one phone whose clock is wrong. So most of these are +/// about what it must still catch, and the rest about what it must not. +library; + +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; + +/// A throwaway tree with [files] under it and the real gate script copied in, +/// so the test cannot drift from what CI runs. +Directory treeWith(Map files) { + final dir = Directory.systemTemp.createTempSync('dpipclock'); + addTearDown(() => dir.deleteSync(recursive: true)); + for (final entry in files.entries) { + File('${dir.path}/${entry.key}') + ..parent.createSync(recursive: true) + ..writeAsStringSync(entry.value); + } + File('${dir.path}/tool/check/wall_clock.sh').parent + .createSync(recursive: true); + File('${Directory.current.path}/tool/check/wall_clock.sh') + .copySync('${dir.path}/tool/check/wall_clock.sh'); + return dir; +} + +ProcessResult gate(Directory tree) => + Process.runSync('bash', ['${tree.path}/tool/check/wall_clock.sh']); + +/// Runs the gate over one file at lib/a.dart and returns what it said. +({int exitCode, String output}) check(String source) { + final result = gate(treeWith({'lib/a.dart': source})); + return ( + exitCode: result.exitCode, + output: '${result.stdout}${result.stderr}', + ); +} + +void main() { + test('an unmarked call fails, by file and line, pointing at the rule', () { + final result = check(''' +void f() { + final t = DateTime.now(); +} +'''); + expect(result.exitCode, 1); + expect(result.output, contains('lib/a.dart:2:')); + expect(result.output, contains('ARCHITECTURE.md → Calibrated time')); + }); + + test('a reason on the same line passes', () { + final result = check(''' +final t = DateTime.now(); // wall-clock: compared only with itself +'''); + expect(result.exitCode, 0, reason: result.output); + }); + + test('a reason anywhere in the comment directly above passes', () { + final result = check(''' +void f() { + // wall-clock: the native side stamps this with the device clock, so its + // age is measured by the same one. + final t = DateTime.now(); +} +'''); + expect(result.exitCode, 0, reason: result.output); + }); + + test('a comment above an annotation still counts', () { + final result = check(''' +class C { + // wall-clock: this is the device clock, by name. + @override + DateTime now() => DateTime.now(); +} +'''); + expect(result.exitCode, 0, reason: result.output); + }); + + test('a bare marker with no reason does not', () { + // The marker is not a password. It is there to make the author decide + // which clock the value is compared with, and an empty one decides nothing. + final result = check(''' +final t = DateTime.now(); // wall-clock: +'''); + expect(result.exitCode, 1); + }); + + test('a marker separated from the call by code does not', () { + final result = check(''' +void f() { + // wall-clock: meant for something else + final a = 1; + final t = DateTime.now(); +} +'''); + expect(result.exitCode, 1); + expect(result.output, contains('lib/a.dart:4:')); + }); + + test('a tear-off is a read of the clock too', () { + final result = check(''' +class C { + C({DateTime Function()? now}) : _now = now ?? DateTime.now; + final DateTime Function() _now; +} +'''); + expect(result.exitCode, 1); + expect(result.output, contains('lib/a.dart:2:')); + }); + + test('DateTime.timestamp() is the same read', () { + final result = check(''' +final t = DateTime.timestamp(); +'''); + expect(result.exitCode, 1); + }); + + test('a call interpolated into a string is still a call', () { + final result = check(r''' +final s = 'at ${DateTime.now()}'; +'''); + expect(result.exitCode, 1); + }); + + test('a // inside a string does not hide the call after it', () { + // A naive "everything after // is a comment" would read the rest of this + // line as a comment and wave the call through. + final result = check(''' +final u = 'https://example.com'; final t = DateTime.now(); +'''); + expect(result.exitCode, 1); + }); + + test('a mention in a comment is not a call', () { + final result = check(''' +/// Never [DateTime.now] for anything a server stamped. +// The device clock — `DateTime.now()`. +final t = 1; // not DateTime.now() +/* DateTime.now() */ +'''); + expect(result.exitCode, 0, reason: result.output); + }); + + test('generated files are skipped', () { + final tree = treeWith({ + 'lib/a.g.dart': 'final t = DateTime.now();\n', + 'lib/b.freezed.dart': 'final t = DateTime.now();\n', + 'lib/l10n/gen/c.dart': 'final t = DateTime.now();\n', + }); + final result = gate(tree); + expect(result.exitCode, 0, reason: '${result.stdout}${result.stderr}'); + }); + + test('every offender is listed, not just the first', () { + final result = check(''' +final a = DateTime.now(); +final b = DateTime.now(); +'''); + expect(result.exitCode, 1); + expect(result.output, contains('lib/a.dart:1:')); + expect(result.output, contains('lib/a.dart:2:')); + }); +} diff --git a/tool/check.sh b/tool/check.sh index 4fac5713e..c8fb54ab9 100755 --- a/tool/check.sh +++ b/tool/check.sh @@ -43,10 +43,10 @@ for gate in workflows workflow_security tooling l10n pubspec_lock \ "tool/check/$gate.sh" done -# These two walk every .dart file in lib/ and take 5.9 s and 2.2 s, which is -# most of what is left once the toolchain steps are cached. Both read lib/ and -# nothing else, so the key is narrow and invalidates exactly when it should. -for gate in layering storage; do +# These walk every .dart file in lib/ and take 5.9 s, 2.2 s and 0.5 s, which +# is most of what is left once the toolchain steps are cached. Each reads lib/ +# and nothing else, so the key is narrow and invalidates exactly when it should. +for gate in layering storage wall_clock; do step "check/$gate" cached "$gate" "$(cache_key lib "tool/check/$gate.sh")" "tool/check/$gate.sh" done diff --git a/tool/check/wall_clock.sh b/tool/check/wall_clock.sh new file mode 100755 index 000000000..bf48b3e64 --- /dev/null +++ b/tool/check/wall_clock.sh @@ -0,0 +1,148 @@ +#!/usr/bin/env bash +# Every read of the device clock in lib/ says why the device clock is right. +# +# tool/check/wall_clock.sh +# +# The device clock is wrong by an unknown amount — set by hand, drifted, or a +# timezone away — and the app measures every server and radio timestamp +# against the SNTP-calibrated `AppTime` instead (ARCHITECTURE.md → Calibrated +# time). A `DateTime.now()` beside one of those timestamps compiles, runs and +# passes its tests on a phone whose clock is right; on one that is not, +# subtracting the two gives the offset between the clocks, not an age. That is +# how a retention window deletes data that is not old, and how an alert's +# countdown is off by however far the phone has drifted. Nothing fails, so +# nothing would ever say so. +# +# Plenty of reads are right, though: a store that both writes and compares its +# own stamps, an OS stamp aged against the clock that made it, the clock being +# calibrated itself. So this does not forbid the call. It requires each one to +# carry its justification, on the same line or in the comment directly above +# (annotations in between are fine): +# +# // wall-clock: LRU sequence key, only ever compared with other rows' +# final now = DateTime.now().millisecondsSinceEpoch; +# +# A reason, not a bare marker — `// wall-clock:` alone does not count. The +# point is that whoever writes the call has to decide which clock it is +# against, and whoever reviews it can check the answer. +# +# `DateTime.timestamp()` is the same read and is held to the same rule. +# Comments are not code, so a `DateTime.now()` mentioned in one is ignored. +# Generated files are skipped. +set -euo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")/../.." + +# python3 owns the scan, as in check/l10n.sh: telling a call from a comment +# needs to know whether a `//` sits inside a string, which a grep cannot. +# Captured, then reported, so the ✗ style matches the other gates. +violations="$(python3 - <<'PY' +import os +import re + +CALL = re.compile(r"\bDateTime\s*\.\s*(now|timestamp)\b") +MARK = re.compile(r"//.*\bwall-clock:\s*\S") +COMMENT_LINE = re.compile(r"^\s*//") +ANNOTATION_LINE = re.compile(r"^\s*@\w") + + +def code_part(line, in_block): + """The line with comments removed, and whether a /* */ is still open. + + Strings are tracked only to know that a `//` or `/*` inside one does not + start a comment; their contents stay, so a call interpolated into a string + is still a call. Escapes and raw strings are close enough for this: a + false start can only hide text after it on the same line, and only inside a + literal that already contains a quote character. + """ + out = [] + i, n = 0, len(line) + quote = None + while i < n: + c = line[i] + if in_block: + if line.startswith("*/", i): + in_block = False + i += 2 + else: + i += 1 + continue + if quote: + out.append(c) + if c == "\\": + if i + 1 < n: + out.append(line[i + 1]) + i += 2 + continue + if line.startswith(quote, i): + out.append(line[i + 1 : i + len(quote)]) + i += len(quote) + quote = None + continue + i += 1 + continue + if line.startswith("//", i): + break + if line.startswith("/*", i): + in_block = True + i += 2 + continue + for q in ("'''", '"""', "'", '"'): + if line.startswith(q, i): + quote = q + out.append(q) + i += len(q) + break + else: + out.append(c) + i += 1 + return "".join(out), in_block + + +def justified(lines, index): + if MARK.search(lines[index]): + return True + # Up through the comment above, stepping over annotations: a comment sits + # above `@override`, not between it and the declaration. + j = index - 1 + while j >= 0 and (COMMENT_LINE.match(lines[j]) or ANNOTATION_LINE.match(lines[j])): + if MARK.search(lines[j]): + return True + j -= 1 + return False + + +for root, dirs, files in os.walk("lib"): + dirs.sort() + for name in sorted(files): + path = os.path.join(root, name) + if not name.endswith(".dart") or name.endswith((".g.dart", ".freezed.dart")): + continue + if path.startswith(os.path.join("lib", "l10n", "gen") + os.sep): + continue + with open(path, encoding="utf-8") as f: + lines = f.read().split("\n") + in_block = False + for index, line in enumerate(lines): + code, in_block = code_part(line, in_block) + if CALL.search(code) and not justified(lines, index): + print(f"{path}:{index + 1}: {line.strip()}") +PY +)" + +if [[ -n $violations ]]; then + while IFS= read -r v; do + printf ' \033[31m✗\033[0m %s\n' "$v" >&2 + done <<<"$violations" + cat >&2 <<'EOF' + +Each of these reads the device clock without saying why that is the right +clock. If the value is compared with anything a server or a radio stamped, +use AppTime instead — see ARCHITECTURE.md → Calibrated time. If the device +clock really is the right one, say why, on the line or in the comment above: + + // wall-clock: +EOF + exit 1 +fi + +echo "wall clock OK — every DateTime.now() in lib/ says why."