diff --git a/README.md b/README.md index 26b0db4..6da4bc0 100644 --- a/README.md +++ b/README.md @@ -21,11 +21,13 @@ here. | script | what it does | |---|---| +| `next-version-code.sh` | print the next monotonic `versionCode` for an application id, read from a repo index -- called before the build, CI and a manual build alike (see "One versionCode rule" below) | | `sign-apk.sh` | check alignment, then sign a prebuilt unsigned release APK with a keystore the caller supplies | | `fdroid-update.sh` | run `fdroid update` over a `repo/` directory of already-signed APKs and produce the signed index | -| `publish.sh` | upload the resulting repo tree to an S3-compatible bucket | +| `publish.sh` | upload the resulting repo tree to an S3-compatible bucket -- files first, index last, and refuses to overwrite an already-published APK (see "Publish ordering and safety" below) | | `with-nixpkgs.sh` | run a command with one or more nixpkgs packages on PATH, pinned to the revision in `nixpkgs-pin` | | `check_zip_alignment.py` | the alignment check `sign-apk.sh` runs before signing (see "Align before sign" below) | +| `next_version_code.py` | the stdlib-only implementation `next-version-code.sh` wraps (index read, arithmetic) | | `action.yml` | the composite action a consumer includes to get all of the above | Deliberately **not** in scope: fetching the APK. That needs an upstream @@ -42,6 +44,13 @@ below for what that costs and why it is still the right call here. - uses: actions/checkout@v4 - id: ci uses: https://git.chaosbit.de/weblib/fdroid-repo-ci@main + +- run: | + version_code=$(bash "${{ steps.ci.outputs.path }}/next-version-code.sh" \ + "$FDROID_REPO_INDEX_URL" "$APPLICATION_ID") + echo "version_code=$version_code" >> "$GITHUB_ENV" + # ... feed $version_code into whatever builds app-release-unsigned.apk + - env: APK_KEYSTORE_PASSWORD: ${{ secrets.APK_KEYSTORE_PASSWORD }} run: | @@ -72,13 +81,73 @@ only one that works while a referenced repo is private -- irrelevant for consuming it, since that one holds the keys. `$APK_KEYSTORE`/`$INDEX_KEYSTORE`/`$FDROID_REPO_ROOT`/`$BUCKET`/ -`$S3_ENDPOINT_URL` above are the consumer's own paths and values -- nothing -in this repo defaults or hints at them. Env var *names* like -`APK_KEYSTORE_PASSWORD` are the consumer's choice too, not a convention this -repo imposes: `sign-apk.sh` and `fdroid-update.sh` each take the name of the -environment variable to read a password from as an argument, specifically so -two different keystores in the same job (the APK key and the separate repo -index key -- see below) never have to share one variable name. +`$S3_ENDPOINT_URL`/`$FDROID_REPO_INDEX_URL`/`$APPLICATION_ID` above are the +consumer's own paths and values -- nothing in this repo defaults or hints at +them. Env var *names* like `APK_KEYSTORE_PASSWORD` are the consumer's choice +too, not a convention this repo imposes: `sign-apk.sh` and `fdroid-update.sh` +each take the name of the environment variable to read a password from as an +argument, specifically so two different keystores in the same job (the APK +key and the separate repo index key -- see below) never have to share one +variable name. + +## One `versionCode` rule, shared by CI and a manual build (fdroid-repo-ci#6) + +Two implementations of "the next version code" is the exact failure mode +`next-version-code.sh` exists to avoid, so both CI and a manual build call +this one script rather than each computing their own: + +``` +candidate = whole minutes since 2026-01-01T00:00:00Z +published = highest versionCode already in the repo index for this applicationId +result = max(candidate, published + 1) +``` + +Minutes, not seconds or raw Unix time: `versionCode` is a signed 32-bit int, +and this scheme does not exhaust it for 4,082 years, against 67 years for +seconds and 11 for Unix time. The clamp against `published` exists because +minute granularity means two builds started in the same minute would +otherwise compute the same number -- and it also covers a clock stepping +backwards or an old commit being rebuilt, both of which would otherwise +produce a number lower than one a device has already seen. + +**The clamp is monotonic against *observed* state, not monotonic by +construction.** It does not close the race where two jobs start in the same +minute, both read the same `published` value, both compute the same next +versionCode, and both build and publish -- the window is the whole build, +not just the clock read, since the version code has to be baked into the APK +before it exists. Nothing in `next-version-code.sh` can prevent that. +`publish.sh`'s APK-existence check (below) is the other half of this: it +cannot stop the wasted build, but it turns the collision from a silent, +permanent wrong state into a loud failure at the last moment, when the +information is finally reliable. + +Takes an index-v2.json URL or local file path, and an application id; +prints the integer and nothing else, so a caller does +`versionCode=$(next-version-code.sh "$URL" "$APPLICATION_ID")`. The URL form +needs no credentials, since both this repo and the F-Droid repo it reads are +public -- the point of weblib-viewer#16's "buildable when CI is down". Fails +loudly (non-zero exit, nothing on stdout) if the index cannot be read; an +application id with no entry at all in the index is not an error -- that is +simply the first-ever publish for a channel, and yields the bare candidate. + +## Publish ordering and safety (fdroid-repo-ci#5, fdroid-repo-ci#6) + +An F-Droid repo is a signed index plus the files it names. `publish.sh` +uploads in two passes, unconditionally: every non-index file first, the +index (`index-v1.json(.asc)`, `index-v2.json`, `entry.json`, `entry.jar`, +matched only at the top level of the tree) last. A publish interrupted +anywhere in the first pass leaves the repo describing the old state a little +longer; it can never leave an index naming files that 404. Credentials are +verified with a real authenticated call (`head-bucket`) before either pass, +rather than merely checking that the environment variables are non-empty -- +an unusable-but-present credential failing halfway through is exactly how a +partial (and potentially index-first) upload happens. + +Before pass 1, `publish.sh` also checks every local `*.apk`'s destination +key with `head-object` and refuses the whole publish if any of them already +exists -- the safeguard `next-version-code.sh`'s clamp cannot provide on its +own (see above). This does not prevent the wasted build behind a same-minute +collision; it prevents the collision from silently corrupting the repo. ## Align before sign, and how "no `zipalign`" was verified @@ -188,12 +257,21 @@ here needs, resolved through the same pin. bash run_tests.sh ``` -42 assertions across five suites, ~20s on a warm nix store: +83 assertions across seven suites offline (~20s on a warm nix store), plus 5 +more that only run against a real bucket (see below): * `test_check_zip_alignment.py` -- pure standard library, no nix, no network at all; builds an aligned and a misaligned fixture zip by hand and checks both directions, including the 4096-byte boundary specific to `.so` entries. +* `test_next_version_code.py` -- pure standard library too, testing + `next_version_code.py` as a subprocess against fixture index-v2.json files + written to a temp dir: no prior build, a lower/equal/higher prior (the + clock-went-backwards case), several versions at once, a version entry with + no versionCode, an application id missing from the index entirely (not an + error), and an unreadable/malformed index (fails loudly, nothing on + stdout). "now" is pinned via an undocumented env var seam so the expected + candidate is a fixed number. * `test_with_nixpkgs.sh` -- offline; every way the pinning wrapper is supposed to refuse rather than silently degrade (missing pin, malformed pin, too few arguments). Does not exercise the happy path of actually @@ -205,14 +283,37 @@ bash run_tests.sh against a throwaway keystore generated fresh per run, real `apksigner` and `fdroid update`. Needs the pinned toolchain to be resolvable (locally cached, or fetched once over the network); nothing beyond that. -* `test_publish.sh` -- argument/credential validation and `--dry-run` - command construction, fully offline, plus one real invocation against a - closed local port (`127.0.0.1:1`, nothing listens there) to prove the - built command genuinely attempts a network call rather than failing on its - own arguments. The actual upload path is out of scope for an offline - suite, matching the constraint that only `publish.sh` needs network in the +* `test_next_version_code.sh` -- the real `next-version-code.sh` CLI, through + `with-nixpkgs.sh`: argument count, a local index file end to end, a + missing application id, a missing/malformed local file, and an + unreachable URL (`http://127.0.0.1:1`, the same closed-port trick + `test_publish.sh` uses) -- proving the wrapper's argv/exit-code/stdout + contract, not just the Python module's arithmetic. +* `test_publish.sh` -- argument/credential validation, `--dry-run` command + construction (now two commands, files then index, and asserted in that + order), and a real invocation against a closed local port + (`127.0.0.1:1`), fully offline. Also runs `test_s3_stub.py`, a + same-repo, HEAD-only local HTTP stub, to prove the credential check is a + real authenticated call (a wrong bucket is rejected) and that an + already-published APK is refused before any upload, with a negative + control proving a non-colliding APK does not trip the same check. The + actual upload path -- and the two-pass *ordering* of real uploads, which + nothing offline can see -- is exercised only by the opt-in live round trip + below, matching the constraint that only `publish.sh` needs network in the first place. +`test_publish.sh`'s live section (skipped by default; needs +`FDROID_CI_LIVE_BUCKET`, `FDROID_CI_LIVE_ENDPOINT` and real credentials) adds +5 more assertions against a real bucket: the round trip itself; the index +object provably absent while the file object is already present at the one +point in the middle of a real publish that the ordering guarantee depends on +(observed via a same-process synchronization hook `publish.sh` exposes only +for this, `PUBLISH_SH_TEST_HOOK_BETWEEN_PASSES` -- deliberately not a poll +racing a background upload from outside, since that is exactly the +flakiness fdroid-repo-ci#5 warned against); both objects reading back +byte-identical; a second publish refused when it would overwrite a +real, already-published APK; and that APK's bytes unchanged afterwards. + No `.gitea/workflows` runs this, on the same reasoning weblib-ci#9 recorded: run it by hand before pushing. If that changes for weblib-ci it should probably change here too, at the same time. diff --git a/action.yml b/action.yml index c37f173..208bce0 100644 --- a/action.yml +++ b/action.yml @@ -1,5 +1,5 @@ -# Makes sign-apk.sh, fdroid-update.sh, publish.sh and with-nixpkgs.sh -# available to a workflow and reports where they are. +# Makes sign-apk.sh, fdroid-update.sh, publish.sh, next-version-code.sh and +# with-nixpkgs.sh available to a workflow and reports where they are. # # Consume it with the **full URL**, the same reason recorded in weblib-ci's # README: `uses: owner/repo@main` resolves against the instance's default @@ -17,15 +17,17 @@ name: F-Droid repo publishing tools description: >- Shared, key-free tooling to align-check and sign a prebuilt APK, run - `fdroid update` over a binary repo, and publish the result to an - S3-compatible bucket. The caller supplies the keys, the bucket and the - APK; this action supplies the pinned toolchain and the scripts. + `fdroid update` over a binary repo, compute the next monotonic versionCode, + and publish the result to an S3-compatible bucket. The caller supplies the + keys, the bucket and the APK; this action supplies the pinned toolchain and + the scripts. outputs: path: description: >- Directory holding with-nixpkgs.sh, nixpkgs-pin, sign-apk.sh, - fdroid-update.sh and publish.sh. + fdroid-update.sh, next-version-code.sh, next_version_code.py and + publish.sh. value: ${{ github.action_path }} runs: @@ -34,6 +36,7 @@ runs: - shell: bash run: | echo "fdroid-repo-ci tools at ${{ github.action_path }}" - for f in with-nixpkgs.sh nixpkgs-pin sign-apk.sh fdroid-update.sh publish.sh; do + for f in with-nixpkgs.sh nixpkgs-pin sign-apk.sh fdroid-update.sh \ + next-version-code.sh next_version_code.py publish.sh; do test -f "${{ github.action_path }}/$f" done diff --git a/next-version-code.sh b/next-version-code.sh new file mode 100755 index 0000000..9d2df65 --- /dev/null +++ b/next-version-code.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# +# One monotonic versionCode rule, shared by CI and manual builds +# (fdroid-repo-ci#6). Two implementations of "the next version code" is the +# whole failure mode this avoids, so both paths call this one script rather +# than each computing their own. +# +# next-version-code.sh +# +# Prints the integer to stdout and nothing else: +# +# versionCode=$(bash next-version-code.sh "$INDEX_URL" "$APPLICATION_ID") +# +# The index source may be an http(s) URL -- what both CI and a manual build +# use in practice, and needs no credentials, since this repo and the F-Droid +# repo it reads are both public (weblib-viewer#16: buildable when CI is +# down) -- or a local file path, which is mainly what the offline test suite +# uses but is equally valid for a build that already has the index checked +# out. +# +# Fails loudly (non-zero exit, message on stderr, nothing on stdout) if the +# index cannot be fetched or parsed -- see next_version_code.py for why that +# is never allowed to fall back to a guess. An index with no entry at all for +# this application id is NOT an error: that is a normal first-ever publish +# for a channel, and yields the bare candidate. +# +# The rule itself, and why each part of it is shaped this way, lives in +# next_version_code.py's own docstring -- read that, not this file, for the +# reasoning. This script only resolves the pinned interpreter and forwards +# arguments; everything that can be unit-tested without nix lives in the +# stdlib-only Python module next to it. +# +# Pins python3 (stdlib json/urllib/datetime/argparse does the rest -- no +# other package) via with-nixpkgs.sh/nixpkgs-pin, next to this script. +set -euo pipefail + +here=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) + +if [ "$#" -ne 2 ]; then + cat >&2 <<'USAGE' +usage: next-version-code.sh +USAGE + exit 2 +fi + +exec bash "$here/with-nixpkgs.sh" python3 python3 "$here/next_version_code.py" "$@" diff --git a/next_version_code.py b/next_version_code.py new file mode 100755 index 0000000..07b5656 --- /dev/null +++ b/next_version_code.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Compute the next monotonic Android versionCode for an application id. + +Implements the rule from fdroid-repo-ci#6 -- read that issue for the reasoning +behind each part; this is only the arithmetic and the index read. + + candidate = whole minutes since 2026-01-01T00:00:00Z + published = highest versionCode already in the index for this app id + result = max(candidate, published + 1) + +Minutes, not seconds or raw Unix time: versionCode is a signed 32-bit int, +and minutes-since-2026 does not exhaust it for 4,082 years. The clamp against +`published` is not optional -- two builds inside the same minute would +otherwise compute the same candidate, and it also covers a clock stepping +backwards or an old commit being rebuilt, both of which would otherwise emit +a number lower than one a device has already seen. + +Reads `packages..versions.*.manifest.versionCode`, the shape +fdroidserver's own index.py actually writes (`packages` keyed by +applicationId/packageName; `versions` a dict keyed by each APK's sha256, not +a list; the versionCode nested under that version's `manifest`) -- verified +against fdroidserver's source, not assumed from an older v1-shaped example. + +An application id absent from the index entirely is not an error: it is the +first-ever publish for a channel, and "published" is then simply nobody, so +the bare candidate is correct. An unreadable or unparseable index IS an +error -- guessing a number when the published state is unknown is exactly +how a duplicate or a decrease gets published. + +Standard library only, deliberately -- no third-party dependency for a script +that reads a signed index. `urllib.request` handles the http(s) case; a local +path is read directly, which is what makes the offline test suite possible +(fixture index-v2.json files, no server, no network). +""" +import argparse +import datetime +import json +import os +import sys +import urllib.request + +EPOCH = datetime.datetime(2026, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) + + +def minutes_since_epoch(now=None): + now = now if now is not None else datetime.datetime.now(datetime.timezone.utc) + return int((now - EPOCH).total_seconds() // 60) + + +def load_index(source, timeout=30): + """source is an http(s) URL or a local file path to index-v2.json.""" + if source.startswith("http://") or source.startswith("https://"): + with urllib.request.urlopen(source, timeout=timeout) as resp: + data = resp.read() + else: + with open(source, "rb") as f: + data = f.read() + return json.loads(data) + + +def highest_published_version_code(index, application_id): + """The highest versionCode already published for application_id, or None + if the index has no entry for it at all (a normal state: the first + publish for this channel).""" + packages = index.get("packages") or {} + package = packages.get(application_id) + if not package: + return None + versions = package.get("versions") or {} + codes = [ + manifest["versionCode"] + for version in versions.values() + if isinstance((manifest := (version or {}).get("manifest")), dict) + and isinstance(manifest.get("versionCode"), int) + ] + return max(codes) if codes else None + + +def next_version_code(source, application_id, now=None): + index = load_index(source) + candidate = minutes_since_epoch(now) + published = highest_published_version_code(index, application_id) + if published is None: + return candidate + return max(candidate, published + 1) + + +def main(argv): + parser = argparse.ArgumentParser( + description="Print the next monotonic versionCode for an application id." + ) + parser.add_argument("index_source", help="index-v2.json URL or local path") + parser.add_argument("application_id") + args = parser.parse_args(argv) + + # Testing seam only, deliberately undocumented in the usage banner: lets + # the offline suite pin "now" so `candidate` is a fixed, known number + # instead of whatever minute the test happens to run in. Unset in every + # real invocation, so production behaviour is untouched. + now = None + now_override = os.environ.get("NEXT_VERSION_CODE_NOW") + if now_override: + now = datetime.datetime.fromisoformat(now_override) + if now.tzinfo is None: + now = now.replace(tzinfo=datetime.timezone.utc) + + try: + result = next_version_code(args.index_source, args.application_id, now=now) + except (OSError, ValueError) as e: + # OSError covers both a local file that can't be read and a network + # failure (urllib.error.URLError is a subclass); ValueError covers + # malformed JSON. Fail loudly on stderr, print nothing on stdout -- + # a caller doing `versionCode=$(next-version-code.sh ...)` must never + # capture a number that was actually an error message. + print(f"next-version-code.py: could not read the index at " + f"{args.index_source!r}: {e}", file=sys.stderr) + return 1 + + print(result) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/publish.sh b/publish.sh index 16e3be6..db91404 100755 --- a/publish.sh +++ b/publish.sh @@ -17,25 +17,97 @@ # backend that ignores it; set AWS_DEFAULT_REGION if the endpoint cares, # otherwise the placeholder below is never actually consulted. # +# ## Ordering (fdroid-repo-ci#5) +# +# An F-Droid repo is a signed index plus the files it names. A partial +# upload can leave them disagreeing in two ways: the index lands first and +# names files that still 404 (breaks the repo for every client until the +# upload finishes or is rolled back), or the files land first and the index +# is not yet updated (harmless -- the new build is merely not visible yet). +# Only the first is dangerous, and `aws s3 sync` walking a tree in its own +# order gives no protection against it. So this script uploads in two +# passes, unconditionally: every non-index file first, the index itself +# last. The repo is therefore always internally consistent -- before a +# publish it still describes the old state, after it describes the new one, +# and an interruption anywhere in pass 1 simply leaves it describing the old +# state a little longer, never a mixture. +# +# "The index" here is index-v1.json(.asc), index-v2.json, entry.json and +# entry.jar -- fdroidserver's actual top-level index/entry file names, +# matched only at the top level of the tree (a nested path like +# diff/.json is not "the index" and travels with the ordinary files; +# it is only ever reached by a client that already has last publish's index +# and is willing to look for it). +# +# ## Credentials are verified before anything is uploaded (fdroid-repo-ci#5) +# +# Checking that AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY are merely *set* does +# not mean they are *usable* -- an unusable-but-present credential failing +# halfway through is exactly how a partial (and, per the ordering point +# above, potentially index-first) upload happens. So before pass 1 this +# script makes one real authenticated call (`head-bucket`) and aborts if it +# fails, rather than discovering that the credentials do not work partway +# through a sync. +# +# ## Refusing to overwrite an existing APK (fdroid-repo-ci#6) +# +# next-version-code.sh's clamp is monotonic against *observed* state, not +# monotonic by construction -- it does not close the race where two jobs +# start in the same minute, both read the same `published` value, both +# compute the same next versionCode, and both build and publish. The window +# is the whole build, not just the clock read, because the version code is +# baked into the APK before it exists. Nothing in this script can prevent +# that wasted build. What it CAN do -- and is required to do, fdroid-repo-ci#6 +# -- is turn the collision from a silent, permanent wrong state (two +# different APKs claiming one versionCode, with clients pinned to whichever +# they happened to fetch first) into a loud failure of the second job, at +# the last moment when the information is finally reliable: immediately +# before that job would otherwise upload over the first job's APK. An +# F-Droid repo names an APK `_.apk`, so the +# collision this guards against is exactly a name collision -- every local +# *.apk file's destination key is checked with `head-object` before pass 1, +# and the whole publish aborts if any of them is already there. This runs +# for every *.apk regardless of whether it actually collides, because there +# is no cheaper way to know in advance which ones might. +# +# Note this also means retrying a publish that already got partway through +# uploading its own APKs (e.g. pass 1 died mid-sync from a network drop) +# will now itself be refused on retry, indistinguishable at the object level +# from a genuine concurrent collision. That is an accepted cost of the same +# trade: this check cannot tell "the same job resuming" from "a different +# job that computed the same versionCode" without hashing every object, +# which was judged not worth the added complexity here. Recovering from that +# case is a fresh publish with a new versionCode, or deleting the partial +# upload by hand. +# # Uses `aws s3 sync`, not `cp --recursive`: re-publishing re-uploads only # what changed. Does **not** pass `--delete` by default -- an F-Droid repo # normally keeps every APK it has ever served (older index entries can still # point at them), so silently pruning the remote on every publish is the # wrong default. Pass `--delete` as an extra argument if a caller genuinely -# wants the remote pruned to match local. +# wants the remote pruned to match local; it is forwarded to *both* passes, +# which is safe because `aws s3 sync --delete` never deletes anything that +# was excluded by that pass's own --exclude/--include filters -- so the +# files-pass never touches an index object and the index-pass never touches +# a file object, regardless of --delete. # # `fdroid update` needs no network; this is the one script in this repo that # does (fdroid-repo-ci#1), and is accordingly the one script whose happy path # the test suite cannot exercise against a real bucket. `--dry-run` prints -# the exact command this script would run -- including the `with-nixpkgs.sh` -# pin -- and exits without touching the network, which is what the test -# suite asserts against. (A PATH-stubbed `aws` cannot do this job: `nix -# shell` puts the packages it was asked for at the *front* of PATH, ahead of -# anything already there, so a stub earlier on the caller's PATH would simply -# be shadowed by the real `aws` from awscli2.) Separately, the test suite -# also runs this for real against a closed local port, to confirm the -# constructed command is well-formed and genuinely attempts a network call -# rather than failing on argument parsing. +# the exact two commands this script would run, in order -- including the +# `with-nixpkgs.sh` pin -- and exits without touching the network at all: no +# credential check, no APK-existence check, no sync. That is what the test +# suite asserts against for command construction and ordering. (A +# PATH-stubbed `aws` cannot do this job: `nix shell` puts the packages it was +# asked for at the *front* of PATH, ahead of anything already there, so a +# stub earlier on the caller's PATH would simply be shadowed by the real +# `aws` from awscli2.) The credential check, the APK-existence check and the +# two-pass ordering are each proven separately: a real invocation against a +# closed local port for "fails on the network, and fails during the +# credential check specifically, before any upload is attempted"; a tiny +# local HEAD-only HTTP stub for "an existing APK is refused, and only an +# existing APK", offline; and the opt-in live round trip against a real +# bucket for the actual upload path plus a real head-to-head APK collision. # # Pins awscli2 via with-nixpkgs.sh/nixpkgs-pin, next to this script. set -euo pipefail @@ -79,6 +151,7 @@ dest="s3://$bucket" if [ -n "$prefix" ]; then dest="$dest/${prefix#/}" fi +region="${AWS_DEFAULT_REGION:-us-east-1}" # awscli2 2.23+ attaches a checksum algorithm to every PutObject by default. # Garage rejects it outright -- measured against a real bucket: @@ -98,16 +171,76 @@ fi export AWS_REQUEST_CHECKSUM_CALCULATION="${AWS_REQUEST_CHECKSUM_CALCULATION:-when_required}" export AWS_RESPONSE_CHECKSUM_VALIDATION="${AWS_RESPONSE_CHECKSUM_VALIDATION:-when_required}" -cmd=(bash "$here/with-nixpkgs.sh" awscli2 aws s3 sync "$local_dir" "$dest" - --endpoint-url "$endpoint" - --region "${AWS_DEFAULT_REGION:-us-east-1}" - "${extra_args[@]}") +aws_base=(bash "$here/with-nixpkgs.sh" awscli2 aws --endpoint-url "$endpoint" --region "$region") + +# Top-level index/entry file names only -- see the "Ordering" comment above +# for why a nested path such as diff/.json is deliberately not matched. +index_exclude_filters=(--exclude "index*" --exclude "entry.*") +index_only_filters=(--exclude "*" --include "index*" --include "entry.*") + +cmd_files=("${aws_base[@]}" s3 sync "$local_dir" "$dest" + "${index_exclude_filters[@]}" "${extra_args[@]}") +cmd_index=("${aws_base[@]}" s3 sync "$local_dir" "$dest" + "${index_only_filters[@]}" "${extra_args[@]}") if [ "$dry_run" = true ]; then - printf '%q ' "${cmd[@]}" + printf '%q ' "${cmd_files[@]}" + printf '\n' + printf '%q ' "${cmd_index[@]}" printf '\n' exit 0 fi -echo "publish.sh: syncing $local_dir -> $dest (endpoint $endpoint)" -exec "${cmd[@]}" +echo "publish.sh: verifying credentials against $endpoint" +if ! verify_out=$("${aws_base[@]}" s3api head-bucket --bucket "$bucket" 2>&1); then + echo "publish.sh: credentials rejected (or bucket unreachable) -- nothing was uploaded:" >&2 + echo "$verify_out" >&2 + exit 1 +fi + +# Refuse to overwrite an existing APK -- see the comment block above for why +# this exists and what it does and does not close. Every local *.apk is +# checked, regardless of nesting, since fdroidserver's binary-repo layout +# keeps them under repo/ directly but nothing here should assume that. +while IFS= read -r -d '' apk; do + rel=${apk#"$local_dir"/} + key="$rel" + if [ -n "$prefix" ]; then + key="${prefix#/}/$rel" + fi + if head_out=$("${aws_base[@]}" s3api head-object --bucket "$bucket" --key "$key" 2>&1); then + echo "publish.sh: refusing to publish -- $key already exists in s3://$bucket (would overwrite an existing APK, fdroid-repo-ci#6)" >&2 + exit 1 + elif echo "$head_out" | grep -qi '404\|not found\|NoSuchKey'; then + : # does not exist yet -- fine, this is the common case + else + echo "publish.sh: could not check whether $key already exists -- refusing to guess:" >&2 + echo "$head_out" >&2 + exit 1 + fi +done < <(find "$local_dir" -type f -name '*.apk' -print0) + +# Test-only synchronization point: if set, run it after the APK-existence +# check and before pass 1, and again between the two passes. Lets the test +# suite observe/assert real bucket state at the one moment that actually +# matters for the ordering guarantee (fdroid-repo-ci#5) -- files present, +# index not yet -- without racing a real background upload from outside. +# Never set by a real caller; not part of the documented interface above. +run_test_hook() { + local hook_var=$1 + local hook_value="${!hook_var:-}" + [ -n "$hook_value" ] && eval "$hook_value" + return 0 +} + +run_test_hook PUBLISH_SH_TEST_HOOK_BEFORE_UPLOAD + +echo "publish.sh: pass 1/2: uploading files (index deferred) $local_dir -> $dest" +"${cmd_files[@]}" + +run_test_hook PUBLISH_SH_TEST_HOOK_BETWEEN_PASSES + +echo "publish.sh: pass 2/2: uploading the index $local_dir -> $dest" +"${cmd_index[@]}" + +echo "publish.sh: done" diff --git a/run_tests.sh b/run_tests.sh index 56a8484..aed150b 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -7,11 +7,13 @@ # # bash run_tests.sh # -# check_zip_alignment.py's own suite needs only python3 (via with-nixpkgs.sh, -# since none is assumed to be on PATH); everything else additionally needs -# apksigner/fdroidserver/jdk21/awscli2, resolved through the same pin. All of -# it stays on loopback/local-store -- nothing here reaches a real host, -# bucket or key. +# check_zip_alignment.py's and next_version_code.py's own suites need only +# python3 (via with-nixpkgs.sh, since none is assumed to be on PATH); +# everything else additionally needs apksigner/fdroidserver/jdk21/awscli2, +# resolved through the same pin. All of it stays on loopback/local-store -- +# nothing here reaches a real host, bucket or key, except the opt-in live +# section inside test_publish.sh, which stays skipped unless +# FDROID_CI_LIVE_BUCKET/FDROID_CI_LIVE_ENDPOINT and real credentials are set. set -uo pipefail here=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -21,9 +23,10 @@ total_fail=0 any_hard_fail=0 run_python_suite() { - echo "=== test_check_zip_alignment.py ===" + local script=$1 + echo "=== $script ===" local out - if out=$(bash "$here/with-nixpkgs.sh" python3 python3 "$here/test_check_zip_alignment.py" -v 2>&1); then + if out=$(bash "$here/with-nixpkgs.sh" python3 python3 "$here/$script" -v 2>&1); then local n n=$(echo "$out" | grep -oE '^Ran [0-9]+ test' | grep -oE '[0-9]+') total_pass=$((total_pass + ${n:-0})) @@ -59,10 +62,12 @@ run_shell_suite() { echo } -run_python_suite +run_python_suite test_check_zip_alignment.py +run_python_suite test_next_version_code.py run_shell_suite test_with_nixpkgs.sh run_shell_suite test_sign_apk.sh run_shell_suite test_fdroid_update.sh +run_shell_suite test_next_version_code.sh run_shell_suite test_publish.sh echo "====================================" diff --git a/test_next_version_code.py b/test_next_version_code.py new file mode 100755 index 0000000..1aab1d6 --- /dev/null +++ b/test_next_version_code.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +"""Offline tests for next_version_code.py (fdroid-repo-ci#6). + +Standard library only, no network, no nix -- like test_check_zip_alignment.py, +this is the piece of the change that needs none of the pinned toolchain. + +Runs next_version_code.py as a *subprocess* against fixture index-v2.json +files written to a temp dir, the same reasoning test_check_zip_alignment.py +gives: a test that only imports the function would miss an argv/exit-code/ +stdout-pollution regression, and "prints the result; do nothing else" is +itself part of the contract (README/issue #6: a caller does +`versionCode=$(next-version-code.sh ...)`, so stray stdout output would be +silently captured as part of the number). + +"now" is pinned via the NEXT_VERSION_CODE_NOW testing seam (see the script's +main()) so `candidate` is a fixed, known integer instead of whatever minute +the suite happens to run in. + +The property every case after the first two actually defends -- and the one +a future refactor could quietly break while still "looking" correct -- is +that the result is *strictly greater than every versionCode already in the +fixture*, not merely equal to some formula. Each clamp case asserts that +directly, not just the arithmetic. +""" +import datetime +import json +import os +import subprocess +import sys +import tempfile +import unittest + +HERE = os.path.dirname(os.path.abspath(__file__)) +SCRIPT = os.path.join(HERE, "next_version_code.py") +APP_ID = "de.example.app" + +EPOCH = datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc) +# An arbitrary, fixed "now": 100 minutes after the epoch this script uses, +# so the expected candidate (100) is known without re-deriving the formula +# under test. +FIXED_NOW = (EPOCH + datetime.timedelta(minutes=100)).isoformat() +CANDIDATE_AT_FIXED_NOW = 100 + + +def index_with_versions(application_id, version_codes): + """Build an index-v2.json fixture with one package with each versionCode + in a separate version entry, matching fdroidserver's own shape: + packages..versions..manifest.versionCode.""" + versions = {} + for i, vc in enumerate(version_codes): + versions[f"fakehash{i}"] = {"manifest": {"versionCode": vc}} + return {"packages": {application_id: {"versions": versions}}} + + +class NextVersionCodeTest(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + + def _write_index(self, name, obj_or_text): + path = os.path.join(self.tmp.name, name) + with open(path, "w") as f: + if isinstance(obj_or_text, str): + f.write(obj_or_text) + else: + json.dump(obj_or_text, f) + return path + + def _run(self, index_path, application_id=APP_ID, now=FIXED_NOW): + env = dict(os.environ) + if now is not None: + env["NEXT_VERSION_CODE_NOW"] = now + else: + env.pop("NEXT_VERSION_CODE_NOW", None) + return subprocess.run( + [sys.executable, SCRIPT, index_path, application_id], + capture_output=True, text=True, env=env, + ) + + # -- no prior build: the first-ever publish for a channel ------------- + + def test_no_prior_build_yields_bare_candidate(self): + path = self._write_index("empty.json", {"packages": {}}) + result = self._run(path) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, f"{CANDIDATE_AT_FIXED_NOW}\n") + + def test_application_id_missing_entirely_is_not_an_error(self): + path = self._write_index( + "other-app.json", index_with_versions("some.other.app", [5])) + result = self._run(path) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, f"{CANDIDATE_AT_FIXED_NOW}\n") + + # -- the clamp: result must exceed every published code ---------------- + + def test_lower_prior_is_ignored_in_favour_of_the_clock(self): + path = self._write_index("lower.json", index_with_versions(APP_ID, [1, 2, 3])) + result = self._run(path) + self.assertEqual(result.returncode, 0, result.stderr) + got = int(result.stdout) + self.assertEqual(got, CANDIDATE_AT_FIXED_NOW) + self.assertGreater(got, 3) + + def test_equal_prior_is_bumped_by_one(self): + # published == candidate: the same-minute collision case. The clamp + # must still move strictly past it. + path = self._write_index( + "equal.json", index_with_versions(APP_ID, [CANDIDATE_AT_FIXED_NOW])) + result = self._run(path) + self.assertEqual(result.returncode, 0, result.stderr) + got = int(result.stdout) + self.assertGreater(got, CANDIDATE_AT_FIXED_NOW) + self.assertEqual(got, CANDIDATE_AT_FIXED_NOW + 1) + + def test_higher_prior_wins_the_clamp_clock_went_backwards(self): + higher = CANDIDATE_AT_FIXED_NOW + 500 + path = self._write_index("higher.json", index_with_versions(APP_ID, [higher])) + result = self._run(path) + self.assertEqual(result.returncode, 0, result.stderr) + got = int(result.stdout) + self.assertGreater(got, higher) + self.assertEqual(got, higher + 1) + + def test_highest_of_several_versions_is_the_one_that_matters(self): + path = self._write_index( + "several.json", index_with_versions(APP_ID, [10, 999, 50])) + result = self._run(path) + self.assertEqual(result.returncode, 0, result.stderr) + got = int(result.stdout) + self.assertGreater(got, 999) + + def test_version_entry_without_a_versionCode_is_ignored_not_fatal(self): + # A version whose manifest lacks versionCode (or has no manifest at + # all) must not crash the read -- it is simply not a candidate for + # "published". + obj = index_with_versions(APP_ID, [7]) + obj["packages"][APP_ID]["versions"]["nomanifest"] = {} + obj["packages"][APP_ID]["versions"]["emptymanifest"] = {"manifest": {}} + path = self._write_index("partial.json", obj) + result = self._run(path) + self.assertEqual(result.returncode, 0, result.stderr) + got = int(result.stdout) + self.assertGreater(got, 7) + + # -- failure modes: fail loudly, never guess --------------------------- + + def test_unreadable_index_fails_loudly(self): + missing = os.path.join(self.tmp.name, "does-not-exist.json") + result = self._run(missing) + self.assertNotEqual(result.returncode, 0) + self.assertEqual(result.stdout, "") # never a guessed number on stdout + self.assertIn("could not read the index", result.stderr) + + def test_malformed_index_fails_loudly(self): + path = self._write_index("malformed.json", "{not valid json") + result = self._run(path) + self.assertNotEqual(result.returncode, 0) + self.assertEqual(result.stdout, "") + self.assertIn("could not read the index", result.stderr) + + # -- output contract: nothing but the integer on stdout ----------------- + + def test_stdout_is_only_the_integer(self): + path = self._write_index("clean.json", {"packages": {}}) + result = self._run(path) + self.assertEqual(result.returncode, 0, result.stderr) + # Exactly one line, and it parses as an int with no surrounding text. + self.assertEqual(int(result.stdout.strip()), CANDIDATE_AT_FIXED_NOW) + self.assertEqual(result.stdout.count("\n"), 1) + + +if __name__ == "__main__": + unittest.main() diff --git a/test_next_version_code.sh b/test_next_version_code.sh new file mode 100755 index 0000000..1a2d07e --- /dev/null +++ b/test_next_version_code.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash +# +# Shell-level integration test for next-version-code.sh (fdroid-repo-ci#6) -- +# the actual CLI entry point, run the same way sign-apk.sh/fdroid-update.sh +# are tested: through with-nixpkgs.sh, against the pinned interpreter, +# exercising real argv/exit-code/stdout behaviour rather than importing a +# function. The arithmetic itself, and every clamp case, is covered offline +# and without nix by test_next_version_code.py; this file only has to prove +# the wrapper script wires that up correctly, plus the two things only the +# wrapper (not the Python module directly) can be asked about: a local file +# path as the index source, and an unreachable URL as one. +# +# bash test_next_version_code.sh +set -euo pipefail +here=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +source "$here/test_lib.sh" + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +APP_ID="de.example.app" + +echo "--- wrong argument count is refused ---" +assert_failure "no arguments" bash "$here/next-version-code.sh" +assert_failure "one argument" bash "$here/next-version-code.sh" "$work/index.json" + +echo "--- a local file path works end to end, through the real CLI ---" +cat > "$work/index.json" < "$work/other-app.json" <<'JSON' +{"packages": {"some.other.app": {"versions": {"h": {"manifest": {"versionCode": 5}}}}}} +JSON +out2=$(bash "$here/next-version-code.sh" "$work/other-app.json" "$APP_ID") +if [[ "$out2" =~ ^[0-9]+$ ]]; then + pass "an application id absent from the index is not an error" +else + fail "an application id absent from the index is not an error (got '$out2')" +fi + +echo "--- a missing local file fails loudly, prints nothing on stdout ---" +out3=$(bash "$here/next-version-code.sh" "$work/does-not-exist.json" "$APP_ID" 2>/tmp/nvc_stderr.$$) && rc=0 || rc=$? +err3=$(cat /tmp/nvc_stderr.$$); rm -f /tmp/nvc_stderr.$$ +if [ "${rc:-0}" -ne 0 ]; then pass "a missing index file exits non-zero"; else + fail "a missing index file exits non-zero (got 0)" +fi +if [ -z "$out3" ]; then pass "a missing index file prints nothing on stdout"; else + fail "a missing index file prints nothing on stdout (got '$out3')" +fi +assert_contains "the error explains the index could not be read" "$err3" "could not read the index" + +echo "--- an unreachable URL fails loudly too, the same way, through the real network path ---" +# port 1 is a reserved, unassigned TCP port -- nothing listens there, so +# this never leaves the loopback interface and needs no real network access, +# the same trick test_publish.sh uses. +out4=$(bash "$here/next-version-code.sh" "http://127.0.0.1:1/index-v2.json" "$APP_ID" 2>/tmp/nvc_stderr2.$$) && rc4=0 || rc4=$? +err4=$(cat /tmp/nvc_stderr2.$$); rm -f /tmp/nvc_stderr2.$$ +if [ "${rc4:-0}" -ne 0 ]; then pass "an unreachable index URL exits non-zero"; else + fail "an unreachable index URL exits non-zero (got 0)" +fi +if [ -z "$out4" ]; then pass "an unreachable index URL prints nothing on stdout"; else + fail "an unreachable index URL prints nothing on stdout (got '$out4')" +fi +assert_contains "the error explains the index could not be read (URL case)" "$err4" "could not read the index" + +echo "--- malformed JSON fails loudly ---" +echo "{not valid json" > "$work/malformed.json" +out5=$(bash "$here/next-version-code.sh" "$work/malformed.json" "$APP_ID" 2>/tmp/nvc_stderr3.$$) && rc5=0 || rc5=$? +err5=$(cat /tmp/nvc_stderr3.$$); rm -f /tmp/nvc_stderr3.$$ +if [ "${rc5:-0}" -ne 0 ]; then pass "malformed JSON exits non-zero"; else + fail "malformed JSON exits non-zero (got 0)" +fi +assert_contains "the error explains the index could not be read (malformed JSON)" "$err5" "could not read the index" + +report_and_exit diff --git a/test_publish.sh b/test_publish.sh index eb3d088..3afcebd 100755 --- a/test_publish.sh +++ b/test_publish.sh @@ -2,13 +2,24 @@ # # Tests for publish.sh -- the one script in this repo that needs real # network to do its actual job (fdroid-repo-ci#1), so its happy path against -# a genuine bucket is out of scope here. What this asserts instead: +# a genuine bucket is out of scope for most of this file. What this asserts: # # * argument/credential validation, offline; -# * `--dry-run` builds the exact command a real run would use, offline; +# * `--dry-run` builds the exact two commands (files, then index) a real run +# would use, in that order, offline; # * a real invocation against a closed local port fails on the network call -# itself, not on argument parsing -- proving the command it built is -# well-formed, without needing a real bucket or reaching beyond loopback. +# itself -- during the credential check specifically, before any upload is +# attempted -- proving both that the constructed command is well-formed +# and that verification genuinely comes first (fdroid-repo-ci#5); +# * a real invocation against a tiny local HEAD-only S3 stub (test_s3_stub.py) +# proves the credential check is a genuine authenticated call (a wrong +# bucket is rejected, not waved through because a variable was merely +# non-empty) and that an already-published APK is refused before any +# upload -- and, as a negative control, that a *non*-colliding APK does +# NOT trip the same check (fdroid-repo-ci#6); +# * the opt-in live round trip against a real bucket, extended to prove the +# two-pass ordering with real uploads (not just command construction) and +# the APK-collision refusal against a real object. set -euo pipefail here=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -23,6 +34,14 @@ LIVE_SECRET="${AWS_SECRET_ACCESS_KEY:-}" LIVE_REGION="${AWS_DEFAULT_REGION:-}" source "$here/test_lib.sh" +PYTHON="python3" +have_python=false +if command -v python3 >/dev/null 2>&1; then + have_python=true +elif py=$(bash "$here/with-nixpkgs.sh" python3 sh -c 'command -v python3' 2>/dev/null); then + have_python=true +fi + work=$(mktemp -d) trap 'rm -rf "$work"' EXIT mkdir -p "$work/repo" @@ -40,7 +59,7 @@ echo "--- missing local directory is refused ---" assert_failure "publish.sh refuses a non-existent local directory" \ bash "$here/publish.sh" "$work/does-not-exist" mybucket https://s3.example.invalid -echo "--- --dry-run builds the right command, touches no network ---" +echo "--- --dry-run builds two commands, files then index, touches no network ---" out=$(bash "$here/publish.sh" --dry-run "$work/repo" mybucket https://s3.example.invalid myprefix) assert_contains "dry-run names the destination bucket and prefix" "$out" "s3://mybucket/myprefix" assert_contains "dry-run names the endpoint" "$out" "--endpoint-url https://s3.example.invalid" @@ -51,11 +70,39 @@ else pass "dry-run does not silently add --delete" fi +nlines=$(echo "$out" | grep -c 's3 sync' || true) +if [ "$nlines" -eq 2 ]; then + pass "dry-run prints exactly two sync commands (files pass, index pass)" +else + fail "dry-run prints exactly two sync commands (files pass, index pass) (got $nlines)" +fi + +files_line=$(echo "$out" | grep -n 's3 sync' | sed -n '1p' | cut -d: -f1) +index_line=$(echo "$out" | grep -n 's3 sync' | sed -n '2p' | cut -d: -f1) +files_cmd=$(echo "$out" | sed -n "${files_line}p") +index_cmd=$(echo "$out" | sed -n "${index_line}p") + +assert_contains "the first (files) pass excludes index-shaped names" "$files_cmd" "--exclude index" +assert_contains "the first (files) pass excludes entry-shaped names" "$files_cmd" "--exclude entry." +if echo "$files_cmd" | grep -q -- "--include"; then + fail "the first (files) pass has no --include (it did!)" +else + pass "the first (files) pass has no --include" +fi +assert_contains "the second (index) pass includes index-shaped names" "$index_cmd" "--include index" +assert_contains "the second (index) pass includes entry-shaped names" "$index_cmd" "--include entry." +assert_contains "the second (index) pass excludes everything else first" "$index_cmd" "--exclude" +if [ "$files_line" -lt "$index_line" ]; then + pass "the files pass is printed before the index pass -- index really does go last" +else + fail "the files pass is printed before the index pass (files at line $files_line, index at $index_line)" +fi + echo "--- --dry-run with no prefix omits the trailing slash entirely ---" out_noprefix=$(bash "$here/publish.sh" --dry-run "$work/repo" mybucket https://s3.example.invalid) assert_contains "no-prefix dry-run targets the bucket root" "$out_noprefix" "s3://mybucket " -echo "--- a real invocation fails on the network call, not on arguments ---" +echo "--- a real invocation fails on the network call, not on arguments -- and fails during credential verification, before any upload ---" # port 1 is a reserved, unassigned TCP port -- nothing listens there, so this # never leaves the loopback interface and needs no real network access. real_out=$(bash "$here/publish.sh" "$work/repo" mybucket "http://127.0.0.1:1" 2>&1) && rc=0 || rc=$? @@ -63,43 +110,230 @@ if [ "${rc:-0}" -ne 0 ]; then pass "publish.sh against a closed port exits non-z fail "publish.sh against a closed port exits non-zero (got 0)" fi assert_contains "the failure is a connection error, not an argument error" "$real_out" "Could not connect" +assert_contains "publish.sh announces credential verification" "$real_out" "verifying credentials" +if echo "$real_out" | grep -q "pass 1/2"; then + fail "publish.sh must not start pass 1 when credential verification never completed (it did!)" +else + pass "publish.sh never starts pass 1 when credential verification fails" +fi + +# --- offline, against a local HEAD-only S3 stub ----------------------------- +# Real S3 semantics (200/404 over HTTP, path-style addressing since the +# endpoint host here is a literal IP) without a real bucket or real +# credentials -- see test_s3_stub.py's own docstring for exactly what it +# does and does not implement, and why that is enough for these two checks +# without ever letting publish.sh reach `aws s3 sync`. +if [ "$have_python" = true ]; then + start_stub() { + # start_stub [ ...] -- prints the chosen port on + # stdout, leaves the server running in the background; caller kills $stub_pid. + local logf + logf=$(mktemp) + bash "$here/with-nixpkgs.sh" python3 python3 "$here/test_s3_stub.py" "$@" \ + >"$logf" 2>&1 & + stub_pid=$! + for _ in $(seq 1 50); do + [ -s "$logf" ] && break + sleep 0.1 + done + stub_port=$(head -1 "$logf") + stub_log=$logf + } + + echo "--- credential verification is a real authenticated call, not just 'is it set' ---" + start_stub realbucket + out=$(bash "$here/publish.sh" "$work/repo" wrongbucket "http://127.0.0.1:$stub_port" 2>&1) && rc=0 || rc=$? + kill "$stub_pid" 2>/dev/null || true + wait "$stub_pid" 2>/dev/null || true + if [ "${rc:-0}" -ne 0 ]; then + pass "publish.sh refuses a bucket the stub does not recognise (bad/unusable credentials)" + else + fail "publish.sh refuses a bucket the stub does not recognise (bad/unusable credentials) (got exit 0)" + fi + assert_contains "the rejection is reported before any upload" "$out" "credentials rejected" + if echo "$out" | grep -q "pass 1/2"; then + fail "publish.sh must not start pass 1 after a rejected credential check (it did!)" + else + pass "publish.sh never starts pass 1 after a rejected credential check" + fi + + echo "--- an already-published APK is refused, before any upload (fdroid-repo-ci#6) ---" + collide_dir="$work/collide" + mkdir -p "$collide_dir" + echo '{}' > "$collide_dir/index-v2.json" + echo 'unsigned bytes racing the real one' > "$collide_dir/de.example.app_1.apk" + start_stub mybucket "myprefix/de.example.app_1.apk" + out=$(bash "$here/publish.sh" "$collide_dir" mybucket "http://127.0.0.1:$stub_port" myprefix 2>&1) && rc=0 || rc=$? + kill "$stub_pid" 2>/dev/null || true + wait "$stub_pid" 2>/dev/null || true + if [ "${rc:-0}" -ne 0 ]; then + pass "publish.sh refuses to publish over an already-existing APK" + else + fail "publish.sh refuses to publish over an already-existing APK (got exit 0)" + fi + assert_contains "the refusal names the colliding key" "$out" "myprefix/de.example.app_1.apk" + assert_contains "the refusal explains why" "$out" "already exists" + if echo "$out" | grep -q "pass 1/2"; then + fail "publish.sh must not start pass 1 after refusing a colliding APK (it did!)" + else + pass "publish.sh never starts pass 1 after refusing a colliding APK" + fi + + echo "--- negative control: a non-colliding APK does not trip the same check ---" + start_stub mybucket # nothing pre-exists + out=$(bash "$here/publish.sh" "$collide_dir" mybucket "http://127.0.0.1:$stub_port" myprefix 2>&1) && rc=0 || rc=$? + kill "$stub_pid" 2>/dev/null || true + wait "$stub_pid" 2>/dev/null || true + # The stub has no sync/PUT support at all, so pass 1 is expected to fail + # once it gets there -- the only thing this proves is that the collision + # check itself did not false-positive and let the script proceed past it. + assert_contains "with nothing pre-existing, publish.sh proceeds to pass 1" "$out" "pass 1/2" +else + skip "credential-check-is-real / APK-collision-refusal (no python3 available, even via with-nixpkgs.sh)" + skip "negative control: non-colliding APK does not trip the check" +fi # --- live round trip, opt-in ------------------------------------------------ # Skipped unless a scratch bucket and credentials are present, so the default -# offline run is unchanged. This is the only test here that talks to a real -# server, and the only one that could have caught the Garage checksum -# rejection (#3): everything above proves the command is well formed, which is -# a different claim from a server accepting it. +# offline run is unchanged. This is the only part of the suite that talks to +# a real server, and the only part that can prove the two-pass *ordering* +# with real uploads rather than command construction, and the APK-collision +# refusal against a real, already-published object. if [ -n "${FDROID_CI_LIVE_BUCKET:-}" ] && [ -n "${FDROID_CI_LIVE_ENDPOINT:-}" ] \ && [ -n "$LIVE_KEY_ID" ] && [ -n "$LIVE_SECRET" ]; then echo "--- a real round trip against a real endpoint ---" export AWS_ACCESS_KEY_ID="$LIVE_KEY_ID" export AWS_SECRET_ACCESS_KEY="$LIVE_SECRET" [ -n "$LIVE_REGION" ] && export AWS_DEFAULT_REGION="$LIVE_REGION" + # publish.sh sets these itself, but only inside its own (child) process -- + # it does not propagate back here, and every direct `aws s3 cp -` *write* + # this file does below needs it too, or it fails exactly like the Garage + # checksum bug documented in publish.sh (InvalidRequest: invalid checksum + # algorithm), which reads like a permissions problem. + export AWS_REQUEST_CHECKSUM_CALCULATION="${AWS_REQUEST_CHECKSUM_CALCULATION:-when_required}" + export AWS_RESPONSE_CHECKSUM_VALIDATION="${AWS_RESPONSE_CHECKSUM_VALIDATION:-when_required}" live_dir=$(mktemp -d) stamp="live-$(date -u +%s)-$$" printf 'publish.sh live probe %s\n' "$stamp" > "$live_dir/probe.txt" + printf '{"probe":"%s"}\n' "$stamp" > "$live_dir/index-v2.json" - if bash "$here/publish.sh" "$live_dir" "$FDROID_CI_LIVE_BUCKET" \ - "$FDROID_CI_LIVE_ENDPOINT" "$stamp" >/dev/null 2>&1; then + cleanup_live() { + bash "$here/with-nixpkgs.sh" awscli2 aws --endpoint-url "$FDROID_CI_LIVE_ENDPOINT" \ + s3 rm --recursive "s3://$FDROID_CI_LIVE_BUCKET/$stamp/" >/dev/null 2>&1 || true + rm -rf "$live_dir" + } + + echo "--- upload ordering: index must not exist while the file already does (the hard part -- see below) ---" + # This is the one property the offline suite structurally cannot see: it + # is about the order two *real* uploads land in, which only a real bucket + # can answer. Racing the check in from outside against a real network call + # is exactly the flakiness fdroid-repo-ci#5 warned off ("worth building the + # check into the script's own output rather than trying to catch a race + # from outside"), so instead this uses the synchronization hook publish.sh + # exposes for tests (PUBLISH_SH_TEST_HOOK_BETWEEN_PASSES): a real HEAD + # against the real bucket runs from *inside* publish.sh's own process, + # at the one moment guaranteed to be after pass 1 (files) finished and + # before pass 2 (index) started -- not a poll racing a background upload. + order_result=$(mktemp) + hook_script=$(mktemp) + cat > "$hook_script" <<'HOOK' +#!/usr/bin/env bash +set -uo pipefail +aws=(bash "$FDROID_HOOK_HERE/with-nixpkgs.sh" awscli2 aws \ + --endpoint-url "$FDROID_HOOK_ENDPOINT" --region "${FDROID_HOOK_REGION:-us-east-1}") +file_present=0 +index_absent=0 +if "${aws[@]}" s3api head-object --bucket "$FDROID_HOOK_BUCKET" \ + --key "$FDROID_HOOK_STAMP/probe.txt" >/dev/null 2>&1; then + file_present=1 +fi +if ! "${aws[@]}" s3api head-object --bucket "$FDROID_HOOK_BUCKET" \ + --key "$FDROID_HOOK_STAMP/index-v2.json" >/dev/null 2>&1; then + index_absent=1 +fi +echo "file_present=$file_present index_absent=$index_absent" > "$FDROID_HOOK_RESULT_FILE" +HOOK + + FDROID_HOOK_HERE="$here" FDROID_HOOK_ENDPOINT="$FDROID_CI_LIVE_ENDPOINT" \ + FDROID_HOOK_REGION="${LIVE_REGION:-us-east-1}" FDROID_HOOK_BUCKET="$FDROID_CI_LIVE_BUCKET" \ + FDROID_HOOK_STAMP="$stamp" FDROID_HOOK_RESULT_FILE="$order_result" \ + PUBLISH_SH_TEST_HOOK_BETWEEN_PASSES="bash '$hook_script'" \ + bash "$here/publish.sh" "$live_dir" "$FDROID_CI_LIVE_BUCKET" \ + "$FDROID_CI_LIVE_ENDPOINT" "$stamp" >/tmp/publish_live_out.$$ 2>&1 + publish_rc=$? + if [ "$publish_rc" -eq 0 ]; then pass "publish.sh uploaded to a real bucket" else - fail "publish.sh uploaded to a real bucket" + fail "publish.sh uploaded to a real bucket (exit $publish_rc)" + cat /tmp/publish_live_out.$$ >&2 fi + rm -f /tmp/publish_live_out.$$ + + if [ -s "$order_result" ]; then + order_line=$(cat "$order_result") + if echo "$order_line" | grep -q "file_present=1 index_absent=1"; then + pass "mid-publish (real bucket): the file was already there and the index was not ($order_line)" + else + fail "mid-publish (real bucket): the file was already there and the index was not (got: $order_line)" + fi + else + fail "the between-passes hook never ran against the real bucket -- ordering was not actually observed" + fi + rm -f "$order_result" "$hook_script" got=$(bash "$here/with-nixpkgs.sh" awscli2 aws --endpoint-url "$FDROID_CI_LIVE_ENDPOINT" \ s3 cp "s3://$FDROID_CI_LIVE_BUCKET/$stamp/probe.txt" - 2>/dev/null || true) if [ "$got" = "publish.sh live probe $stamp" ]; then - pass "the uploaded object reads back byte-identical" + pass "the uploaded file object reads back byte-identical" else - fail "the uploaded object reads back byte-identical" + fail "the uploaded file object reads back byte-identical" + fi + got_index=$(bash "$here/with-nixpkgs.sh" awscli2 aws --endpoint-url "$FDROID_CI_LIVE_ENDPOINT" \ + s3 cp "s3://$FDROID_CI_LIVE_BUCKET/$stamp/index-v2.json" - 2>/dev/null || true) + if [ "$got_index" = "{\"probe\":\"$stamp\"}" ]; then + pass "the uploaded index object (pass 2) reads back byte-identical" + else + fail "the uploaded index object (pass 2) reads back byte-identical" + fi + + echo "--- APK-collision refusal against a real, already-published object ---" + apk_stamp="live-apk-$(date -u +%s)-$$" + apk_key="$apk_stamp/de.example.app_1.apk" + original_bytes="original apk bytes $apk_stamp" + printf '%s' "$original_bytes" | bash "$here/with-nixpkgs.sh" awscli2 aws \ + --endpoint-url "$FDROID_CI_LIVE_ENDPOINT" --region "${LIVE_REGION:-us-east-1}" \ + s3 cp - "s3://$FDROID_CI_LIVE_BUCKET/$apk_key" >/dev/null 2>&1 + + race_dir=$(mktemp -d) + echo '{}' > "$race_dir/index-v2.json" + printf 'a different build racing the same versionCode' > "$race_dir/de.example.app_1.apk" + + race_out=$(bash "$here/publish.sh" "$race_dir" "$FDROID_CI_LIVE_BUCKET" \ + "$FDROID_CI_LIVE_ENDPOINT" "$apk_stamp" 2>&1) && race_rc=0 || race_rc=$? + if [ "${race_rc:-0}" -ne 0 ]; then + pass "publish.sh refuses a second publish that would overwrite a real, already-published APK" + else + fail "publish.sh refuses a second publish that would overwrite a real, already-published APK (got exit 0)" + fi + assert_contains "the real-bucket refusal names the colliding key" "$race_out" "$apk_key" + + after_bytes=$(bash "$here/with-nixpkgs.sh" awscli2 aws --endpoint-url "$FDROID_CI_LIVE_ENDPOINT" \ + s3 cp "s3://$FDROID_CI_LIVE_BUCKET/$apk_key" - 2>/dev/null || true) + if [ "$after_bytes" = "$original_bytes" ]; then + pass "the original APK's bytes are unchanged after the refused publish" + else + fail "the original APK's bytes are unchanged after the refused publish" fi bash "$here/with-nixpkgs.sh" awscli2 aws --endpoint-url "$FDROID_CI_LIVE_ENDPOINT" \ - s3 rm --recursive "s3://$FDROID_CI_LIVE_BUCKET/$stamp/" >/dev/null 2>&1 || true - rm -rf "$live_dir" + s3 rm --recursive "s3://$FDROID_CI_LIVE_BUCKET/$apk_stamp/" >/dev/null 2>&1 || true + rm -rf "$race_dir" + + cleanup_live else skip "live round trip (set FDROID_CI_LIVE_BUCKET, FDROID_CI_LIVE_ENDPOINT and AWS credentials)" + skip "live upload ordering (file present, index absent mid-publish)" + skip "live APK-collision refusal against a real object" fi report_and_exit diff --git a/test_s3_stub.py b/test_s3_stub.py new file mode 100755 index 0000000..a654ca4 --- /dev/null +++ b/test_s3_stub.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python3 +"""Minimal path-style S3 HEAD-only stub server, used only by test_publish.sh +to test publish.sh's credential check and APK-existence check entirely +offline -- no real bucket, no real credentials, nothing beyond loopback. + +Only implements what those two checks actually call: + + HEAD / -- head-bucket, used to verify credentials + HEAD // -- head-object, used to check for an existing APK + +`aws`'s S3 client only uses path-style addressing (rather than +`.` virtual-hosted style) when the endpoint host is a literal +IP address, which `--endpoint-url http://127.0.0.1:` is -- so no +`--endpoint-url` or config trickery is needed to get requests shaped this +way. + +Existing objects are given on the command line; every other key 404s, as +does any bucket name other than the one given -- so a test that passes the +wrong bucket name notices, rather than getting a silent "yes" back. + +Deliberately never handles PUT/GET/LIST: publish.sh must never reach the +`aws s3 sync` calls in a test that uses this stub, since both a rejected +credential and an existing APK are supposed to abort before any upload is +attempted. If a future test needs sync to succeed against this stub, that is +new scope for this file, not an oversight here. + + test_s3_stub.py [ ...] + +Prints the bound port as the one line on stdout, then serves until killed. +""" +import http.server +import socketserver +import sys + + +def main(): + bucket = sys.argv[1] + existing = set(sys.argv[2:]) + + class Handler(http.server.BaseHTTPRequestHandler): + def do_HEAD(self): + path = self.path.lstrip("/") + parts = path.split("/", 1) + if not parts or parts[0] != bucket: + self.send_response(404) + self.end_headers() + return + if len(parts) == 1 or parts[1] == "": + # head-bucket: the one bucket named on the command line + # always "exists" and is reachable with these "credentials". + self.send_response(200) + self.end_headers() + return + key = parts[1] + self.send_response(200 if key in existing else 404) + self.end_headers() + + def log_message(self, fmt, *args): + pass # keep test output quiet; nothing here is diagnostic + + class Server(socketserver.TCPServer): + allow_reuse_address = True + + with Server(("127.0.0.1", 0), Handler) as httpd: + print(httpd.server_address[1], flush=True) + httpd.serve_forever() + + +if __name__ == "__main__": + main()