publish.sh: verify credentials and upload the index last; add next-version-code.sh

Closes #5
Closes #6

An F-Droid repo is a signed index plus the files it names, and a partial
upload could previously leave them disagreeing: aws s3 sync walks the tree
in its own order, so an interrupted publish could land the index before the
APKs it names, breaking the repo for every client until the upload finished
or was rolled back. publish.sh now uploads in two passes, unconditionally --
every non-index file first, the index (index-v1.json(.asc), index-v2.json,
entry.json, entry.jar) last -- so an interruption anywhere in pass 1 simply
leaves the repo describing the old state a little longer, never a mixture.
Credentials are also verified with one real authenticated call
(head-bucket) before either pass, rather than merely checking the
environment variables are non-empty, since an unusable-but-present
credential failing halfway through is exactly how a partial upload happens.

next-version-code.sh is the one implementation of the versionCode rule
(candidate = minutes since 2026-01-01Z, clamped to strictly exceed whatever
is already published for that applicationId), shared by CI and manual
builds so there is exactly one place this can drift. bit pointed out
(appended to #6) that the clamp is monotonic against *observed* state, not
monotonic by construction: two jobs starting in the same minute can both
read the same published value and race to publish the same versionCode on
different APKs. Nothing can prevent that wasted build, so publish.sh now
also refuses to overwrite an already-existing APK object before pass 1 --
turning the collision from a silent, permanent wrong state into a loud
failure at the last moment the information is reliable.

Tested offline (83 assertions: pure-stdlib suites for both scripts, the
real CLIs through with-nixpkgs.sh, and a small local HEAD-only S3 stub
proving the credential check is a real authenticated call and the
APK-collision refusal fires only when it should) and live against
fdroid-weblib-dev (7 more: a real publish observed mid-flight via a
same-process synchronization hook to prove the index is genuinely absent
while the file is already present, both objects reading back
byte-identical, and a second publish refused against a real
already-published APK with its bytes left unchanged).

Co-authored-by: bit <bit@das-labor.org>

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/<ts>.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 `<applicationId>_<versionCode>.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/<ts>.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"