diff --git a/.ai/prompts/implement.md b/.ai/prompts/implement.md index c0f499f..a7a161a 100644 --- a/.ai/prompts/implement.md +++ b/.ai/prompts/implement.md @@ -35,8 +35,10 @@ necessary for release-please to pick up our squash merge commits to main. git checkout -b {{ issue.identifier | lower }}- ``` 4. Implement the changes with clean, logical commits. -5. Run the full quality suite: +5. Run the local quality gate (fast, no network required after deps are installed): - mix precommit + For full CI-equivalent assurance before opening a PR (runs audits and + validation that require network access), use `mix ci` instead. 6. Fix any failures before proceeding. 7. Push the branch and create a PR: ``` diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 74a23a4..5ce0de8 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -50,7 +50,7 @@ jobs: - uses: actions/checkout@v7 with: - # mix precommit calls ci/validate_commit_range.sh, which validates + # mix ci calls ci/validate_commit_range.sh, which validates # every commit since the merge base. A full-history checkout lets the # script resolve that base locally without a second network fetch. fetch-depth: 0 @@ -80,12 +80,13 @@ jobs: app/_build key: ${{ runner.os }}-mix-${{ hashFiles('app/mix.lock') }} - - # mix precommit runs the full 11-step gate: - # 1. ci/validate_pull_request_title.sh — PR title (when required) - # 2. ci/validate_commit_range.sh — all commit subjects in the range - # 3-4. Root format + test (the repo-management tooling itself) - # 5-11. App deps.get, hex.audit, deps.audit, format, credo, - # usage_rules.sync, and test. + # mix ci is the complete integration gate: + # 1. App deps.get (bootstrap) + # 2. ci/validate_pull_request_title.sh — PR title (when required) + # 3. ci/validate_commit_range.sh — all commit subjects in the range + # 4. mix precommit — root format + test, app format + credo + test --exclude ci_only + # 5-7. App hex.audit, deps.audit, usage_rules.sync + # 8. App mix test (the unfiltered suite, including ci_only tests) # BASE_REF carries the exact comparison base SHA so the commit-range # validator never has to guess. For pull_request events it is the exact # base SHA; for push events it is github.event.before; for @@ -105,7 +106,7 @@ jobs: PULL_REQUEST_TITLE: >- ${{ inputs.pull_request_title != '' && inputs.pull_request_title || github.event.pull_request.title }} - run: mix precommit + run: mix ci working-directory: . burrito_changes: @@ -197,4 +198,3 @@ jobs: name: Test the packaged binary across users timeout-minutes: 2 run: ../ci/test_burrito_shared_loader.sh ./burrito_out/lc_linux_x86_64 - diff --git a/AGENTS.md b/AGENTS.md index 2d54ed6..2095aa1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -128,9 +128,19 @@ not need a structural-doc update. change** whenever an Ash resource, action, code interface, association attribute, or shared helper is added, removed, renamed, or materially changed. +- GitHub Update-branch validation decision: + documents/github-update-branch-validation-decision.adoc — canonical + security rationale and required regression behavior for the narrowly scoped + exception that permits GitHub's trusted **Update branch** merge commits. + **Must be read and preserved** whenever changing commit-subject validation, + `ci/validate_commit_range.sh`, Git hooks, or CI quality-gate orchestration. ## Standards +- Bash error handling: never use `set -e`, `set -u`, or `set -o pipefail` + (including combined forms such as `set -euo pipefail`). Handle every command + that can fail with an explicit status check, diagnostic, and exit path. See + `documents/style/bash.adoc` for the required Bash style. - Conventional Commits: app/usage-rules.md — enforced by the `commit-msg` and `pre-push` hooks at `git-hooks/` (run `mix setup` once per clone to activate them). diff --git a/Readme.adoc b/Readme.adoc index 407ac77..f9773e2 100644 --- a/Readme.adoc +++ b/Readme.adoc @@ -461,7 +461,8 @@ $ lproj list --mine == Development First, activate the repo's git hooks (enforces conventional-commit subjects -on every commit, and again on every commit about to be pushed): +on every commit and push, and runs a Hex dependency security audit before a +push): [source,sh] ---- @@ -479,13 +480,24 @@ $ mix lc whoami $ mix lc issue list --output json ---- -The project uses ExUnit and `mix format`. Run the full quality gate with: +The project uses ExUnit and `mix format`. During development, run the fast +local gate repeatedly (format, static analysis, unit tests — no network +required after a one-time `mix deps.get` inside `app/`): [source,sh] ---- $ mix precommit ---- +Before opening a pull request, or to reproduce CI locally, run the complete +integration gate (includes dependency audits, PR-title and commit-range +validation, and any CI-classified tests): + +[source,sh] +---- +$ mix ci +---- + To run only the app test suite or format check directly: [source,sh] diff --git a/app/test/test_helper.exs b/app/test/test_helper.exs index dff1a1e..d5afdf7 100644 --- a/app/test/test_helper.exs +++ b/app/test/test_helper.exs @@ -26,4 +26,6 @@ Application.put_env(:elixir, :ansi_enabled, true) # creates a profile. Application.fetch_env!(:linear_cli, :profiles_db_path) |> File.rm() +# Test selection belongs to the invoking quality gate. `mix precommit` excludes +# `:ci_only`; `mix ci` is unfiltered so it runs the complete suite. ExUnit.start() diff --git a/app/usage-rules.md b/app/usage-rules.md index 013e9ef..248426c 100644 --- a/app/usage-rules.md +++ b/app/usage-rules.md @@ -14,8 +14,9 @@ - Enforced locally by the `commit-msg` hook at `git-hooks/commit-msg` (each commit's own subject, via `ci/validate_conventional_subject.sh`) and the `pre-push` hook at `git-hooks/pre-push` (every non-deletion ref update, - via `ci/validate_push_refs.sh`) — run `mix setup` once per clone to - activate both. + via `ci/validate_push_refs.sh`, followed by `ci/hex-audit.sh`) — run + `mix setup` once per clone to activate both. The Hex audit needs network + access and prevents a push when it finds a vulnerable or retired package. - Enforced in CI across a whole PR's commit range by `ci/validate_commit_range.sh` (skips GitHub's own auto-generated update-branch merge commits). diff --git a/ci/hex-audit.sh b/ci/hex-audit.sh new file mode 100755 index 0000000..1819ee5 --- /dev/null +++ b/ci/hex-audit.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +# Runs Hex's dependency security audit from the app Mix project. This is used +# by the pre-push hook as well as the CI quality gate. + +if ! repo_top=$(git rev-parse --show-toplevel 2>&1) +then + printf 'ERROR: unable to resolve repository root: %s\n' "$repo_top" >&2 + exit 1 +fi + +app_dir="$repo_top/app" + +if [ ! -d "$app_dir" ] +then + printf 'ERROR: app Mix project not found: %s\n' "$app_dir" >&2 + exit 1 +fi + +if ! cd "$app_dir" +then + printf 'ERROR: unable to change to app Mix project: %s\n' "$app_dir" >&2 + exit 1 +fi + +if ! command -v mix >/dev/null 2>&1 +then + printf 'ERROR: mix is not available on PATH\n' >&2 + exit 1 +fi + +exec mix hex.audit diff --git a/ci/patch_musl_nifs.sh b/ci/patch_musl_nifs.sh new file mode 100755 index 0000000..81f80bc --- /dev/null +++ b/ci/patch_musl_nifs.sh @@ -0,0 +1,74 @@ +#!/bin/sh +# Runs inside Alpine to patch the supplied musl NIFs and bundle their libgcc +# runtimes. Arguments are alternating NIF paths and replacement libgcc names. + +die() { + printf 'ERROR: %s\n' "$*" >&2 + exit 1 +} + +contains_line() { + value=$1 + expected=$2 + + if printf '%s\n' "$value" | grep -Fxq "$expected" + then + return 0 + fi + + return 1 +} + +if [ "$#" -eq 0 ] +then + die "expected one or more NIF/libgcc-name pairs" +fi + +apk add --no-cache libgcc patchelf || die "unable to install Alpine patching tools" + +while [ "$#" -gt 0 ] +do + nif=$1 + libgcc_name=$2 + shift 2 + + bundled_libgcc="${nif%/*}/$libgcc_name" + install -m 0755 /usr/lib/libgcc_s.so.1 "$bundled_libgcc" || die "unable to install libgcc for $nif" + + # libc.so is the musl dependency name; glibc NIFs require libc.so.6. + # Check this before patching so a host artifact cannot slip through. + needed=$(patchelf --print-needed "$nif") || die "unable to inspect dependencies for $nif" + + if ! contains_line "$needed" libc.so + then + die "musl NIF does not depend on libc.so: $nif" + fi + + # Set RUNPATH before growing DT_NEEDED. With patchelf 0.18, doing these + # two mutations in the opposite order can produce a loadable NIF that + # crashes on its first call. + patchelf --set-rpath '$ORIGIN' "$nif" || die "unable to set RUNPATH for $nif" + needed=$(patchelf --print-needed "$nif") || die "unable to inspect libgcc dependency for $nif" + + if contains_line "$needed" libgcc_s.so.1 + then + patchelf --replace-needed libgcc_s.so.1 "$libgcc_name" "$nif" || die "unable to replace libgcc dependency for $nif" + elif ! contains_line "$needed" "$libgcc_name" + then + die "musl NIF has no expected libgcc dependency: $nif" + fi + + needed=$(patchelf --print-needed "$nif") || die "unable to verify libgcc dependency for $nif" + + if ! contains_line "$needed" "$libgcc_name" + then + die "musl NIF did not retain renamed libgcc dependency: $nif" + fi + + rpath=$(patchelf --print-rpath "$nif") || die "unable to inspect RUNPATH for $nif" + + if [ "$rpath" != '$ORIGIN' ] + then + die "musl NIF RUNPATH is not \$ORIGIN: $nif" + fi +done diff --git a/ci/prepare_musl_nifs.sh b/ci/prepare_musl_nifs.sh index 96f65d4..778d4df 100755 --- a/ci/prepare_musl_nifs.sh +++ b/ci/prepare_musl_nifs.sh @@ -1,9 +1,12 @@ #!/usr/bin/env bash -set -euo pipefail +die() { + printf 'ERROR: %s\n' "$*" >&2 + exit 1 +} -script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) -repo_root=$(cd -- "$script_dir/.." && pwd) +script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) || die "unable to resolve script directory" +repo_root=$(cd -- "$script_dir/.." && pwd) || die "unable to resolve repository root" build_lib_dir=${1:-"$repo_root/app/_build/prod/lib"} mdex_native_dir="$build_lib_dir/mdex_native/priv/native" @@ -12,43 +15,73 @@ syntect_priv_dir="$build_lib_dir/makeup_syntect/priv" # Mix links a dependency's priv directory back into deps/ by default. Replace # that production-build symlink with a private copy before swapping NIFs, or a # local release build would also replace the glibc NIF used by dev/test. -if [ -L "$syntect_priv_dir" ]; then - syntect_priv_source=$(readlink -f -- "$syntect_priv_dir") - isolated_priv=$(mktemp -d "$build_lib_dir/makeup_syntect/.priv.XXXXXX") +if [ -L "$syntect_priv_dir" ] +then + syntect_priv_source=$(readlink -f -- "$syntect_priv_dir") || die "unable to resolve makeup_syntect priv symlink: $syntect_priv_dir" + isolated_priv=$(mktemp -d "$build_lib_dir/makeup_syntect/.priv.XXXXXX") || die "unable to create isolated makeup_syntect priv directory" + + if ! cp -a "$syntect_priv_source/." "$isolated_priv/" + then + if ! rm -rf -- "$isolated_priv" + then + printf 'WARNING: unable to remove incomplete isolated priv directory: %s\n' "$isolated_priv" >&2 + fi + die "unable to copy makeup_syntect priv directory: $syntect_priv_source" + fi - if ! cp -a "$syntect_priv_source/." "$isolated_priv/"; then - rm -rf -- "$isolated_priv" - exit 1 + if ! unlink "$syntect_priv_dir" + then + if ! rm -rf -- "$isolated_priv" + then + printf 'WARNING: unable to remove isolated priv directory: %s\n' "$isolated_priv" >&2 + fi + die "unable to remove production makeup_syntect priv symlink: $syntect_priv_dir" fi - unlink "$syntect_priv_dir" - mv "$isolated_priv" "$syntect_priv_dir" + if ! mv "$isolated_priv" "$syntect_priv_dir" + then + if ! rm -rf -- "$isolated_priv" + then + printf 'WARNING: unable to remove isolated priv directory: %s\n' "$isolated_priv" >&2 + fi + die "unable to install isolated makeup_syntect priv directory: $syntect_priv_dir" + fi fi syntect_native_dir="$syntect_priv_dir/native" -for native_dir in "$mdex_native_dir" "$syntect_native_dir"; do - if [ ! -d "$native_dir" ]; then +for native_dir in "$mdex_native_dir" "$syntect_native_dir" +do + if [ ! -d "$native_dir" ] + then printf 'native directory does not exist: %s\n' "$native_dir" >&2 exit 1 fi done -mdex_native_dir=$(cd -- "$mdex_native_dir" && pwd -P) -syntect_native_dir=$(cd -- "$syntect_native_dir" && pwd -P) +mdex_native_dir=$(cd -- "$mdex_native_dir" && pwd -P) || die "unable to resolve mdex_native directory" +syntect_native_dir=$(cd -- "$syntect_native_dir" && pwd -P) || die "unable to resolve makeup_syntect directory" -shopt -s nullglob +if ! shopt -s nullglob +then + die "unable to enable nullglob" +fi mdex_nifs=("$mdex_native_dir"/libmdex_native_nif-*-unknown-linux-musl.so) syntect_host_nifs=("$syntect_native_dir"/libmakeup_syntect-*-unknown-linux-gnu.so) -shopt -u nullglob +if ! shopt -u nullglob +then + die "unable to disable nullglob" +fi -if [ "${#mdex_nifs[@]}" -ne 1 ]; then +if [ "${#mdex_nifs[@]}" -ne 1 ] +then printf 'expected exactly one mdex_native musl NIF in %s; found %d\n' \ "$mdex_native_dir" "${#mdex_nifs[@]}" >&2 exit 1 fi -if [ "${#syntect_host_nifs[@]}" -ne 1 ]; then +if [ "${#syntect_host_nifs[@]}" -ne 1 ] +then printf 'expected exactly one makeup_syntect host NIF in %s; found %d\n' \ "$syntect_native_dir" "${#syntect_host_nifs[@]}" >&2 exit 1 @@ -60,11 +93,12 @@ fi # then replace the host NIF (at the path embedded in its BEAM module) with the # checksummed musl release artifact. syntect_host_nif=${syntect_host_nifs[0]} -syntect_host_name=$(basename -- "$syntect_host_nif") +syntect_host_name=$(basename -- "$syntect_host_nif") || die "unable to determine makeup_syntect NIF filename" syntect_musl_name=${syntect_host_name/unknown-linux-gnu/unknown-linux-musl} syntect_archive="$syntect_musl_name.tar.gz" -if [[ ! "$syntect_musl_name" =~ ^libmakeup_syntect-v([^-]+)- ]]; then +if [[ ! "$syntect_musl_name" =~ ^libmakeup_syntect-v([^-]+)- ]] +then printf 'could not determine makeup_syntect version from %s\n' "$syntect_host_name" >&2 exit 1 fi @@ -72,58 +106,79 @@ fi syntect_version=${BASH_REMATCH[1]} checksum_file="$repo_root/app/deps/makeup_syntect/checksum-Elixir.MakeupSyntect.exs" -if [ ! -f "$checksum_file" ]; then +if [ ! -f "$checksum_file" ] +then printf 'makeup_syntect checksum file does not exist: %s\n' "$checksum_file" >&2 exit 1 fi -expected_checksum=$( +if ! expected_checksum=$( elixir -e ' [path, artifact] = System.argv() {checksums, _bindings} = Code.eval_file(path) IO.write(Map.fetch!(checksums, artifact)) ' -- "$checksum_file" "$syntect_archive" ) +then + die "unable to read checksum for $syntect_archive" +fi expected_checksum=${expected_checksum#sha256:} -temp_dir=$(mktemp -d) +temp_dir=$(mktemp -d) || die "unable to create temporary directory" cleanup() { - rm -rf -- "$temp_dir" + if ! rm -rf -- "$temp_dir" + then + printf 'WARNING: unable to remove temporary directory: %s\n' "$temp_dir" >&2 + fi } trap cleanup EXIT syntect_url="https://github.com/elixir-makeup/makeup_syntect/releases/download/v${syntect_version}/${syntect_archive}" downloaded_archive="$temp_dir/$syntect_archive" -curl --fail --location --silent --show-error --retry 3 \ +if ! curl --fail --location --silent --show-error --retry 3 \ --output "$downloaded_archive" "$syntect_url" +then + die "unable to download $syntect_url" +fi -actual_checksum=$(sha256sum "$downloaded_archive" | cut -d ' ' -f 1) +checksum_output=$(sha256sum "$downloaded_archive") || die "unable to calculate checksum for $downloaded_archive" +actual_checksum=${checksum_output%% *} -if [ "$actual_checksum" != "$expected_checksum" ]; then +if [ "$actual_checksum" != "$expected_checksum" ] +then printf 'checksum mismatch for %s\nexpected: %s\nactual: %s\n' \ "$syntect_archive" "$expected_checksum" "$actual_checksum" >&2 exit 1 fi -tar -xzf "$downloaded_archive" -C "$temp_dir" +if ! tar -xzf "$downloaded_archive" -C "$temp_dir" +then + die "unable to extract $downloaded_archive" +fi downloaded_nif="$temp_dir/$syntect_musl_name" -if [ ! -f "$downloaded_nif" ]; then +if [ ! -f "$downloaded_nif" ] +then printf 'makeup_syntect archive did not contain %s\n' "$syntect_musl_name" >&2 exit 1 fi -install -m 0755 "$downloaded_nif" "$syntect_host_nif" +if ! install -m 0755 "$downloaded_nif" "$syntect_host_nif" +then + die "unable to install makeup_syntect musl NIF: $syntect_host_nif" +fi # Rust's dynamically linked musl cdylibs depend on libgcc_s. Burrito starts # its Linux ERTS with a musl loader but puts the host library directories on # LD_LIBRARY_PATH, where Ubuntu's glibc libgcc_s may be found first. Give each # NIF a uniquely named Alpine libgcc runtime beside it and patch DT_NEEDED and # RUNPATH so the loader selects that copy deterministically. -if command -v podman >/dev/null 2>&1 && podman info >/dev/null 2>&1; then +if command -v podman >/dev/null 2>&1 && podman info >/dev/null 2>&1 +then container_runtime=podman -elif command -v docker >/dev/null 2>&1 && docker info >/dev/null 2>&1; then +elif command -v docker >/dev/null 2>&1 && docker info >/dev/null 2>&1 +then container_runtime=docker else printf 'podman or docker is required to prepare the musl NIFs\n' >&2 @@ -132,52 +187,25 @@ fi container_args=(run --rm) -if [ "$container_runtime" = podman ]; then +if [ "$container_runtime" = podman ] +then container_args+=(--security-opt label=disable) fi container_args+=( -v "$mdex_native_dir:/mdex_native" -v "$syntect_native_dir:/makeup_syntect" + -v "$repo_root/ci/patch_musl_nifs.sh:/patch_musl_nifs.sh:ro" ) mdex_nif_name=$(basename -- "${mdex_nifs[0]}") -# The single-quoted body is intentionally expanded by the container's shell. -# shellcheck disable=SC2016 -"$container_runtime" "${container_args[@]}" alpine:3.22 sh -euxc ' - apk add --no-cache libgcc patchelf - - while [ "$#" -gt 0 ]; do - nif="$1" - libgcc_name="$2" - shift 2 - - bundled_libgcc="${nif%/*}/$libgcc_name" - install -m 0755 /usr/lib/libgcc_s.so.1 "$bundled_libgcc" - - # libc.so is the musl dependency name; glibc NIFs require libc.so.6. - # Check this before patching so a host artifact cannot slip through. - patchelf --print-needed "$nif" | grep -Fxq libc.so - - # Set RUNPATH before growing DT_NEEDED. With patchelf 0.18, doing these - # two mutations in the opposite order can produce a loadable NIF that - # crashes on its first call. - patchelf --set-rpath "\$ORIGIN" "$nif" - - if patchelf --print-needed "$nif" | grep -Fxq libgcc_s.so.1; then - patchelf --replace-needed libgcc_s.so.1 "$libgcc_name" "$nif" - elif ! patchelf --print-needed "$nif" | grep -Fxq "$libgcc_name"; then - printf "musl NIF has no expected libgcc dependency: %s\n" "$nif" >&2 - exit 1 - fi - - patchelf --print-needed "$nif" | grep -Fxq "$libgcc_name" - test "$(patchelf --print-rpath "$nif")" = "\$ORIGIN" - done -' sh \ +if ! "$container_runtime" "${container_args[@]}" alpine:3.22 sh /patch_musl_nifs.sh \ "/mdex_native/$mdex_nif_name" libmdex_musl_libgcc_s.so.1 \ "/makeup_syntect/$syntect_host_name" libmakeup_syntect_musl_libgcc_s.so.1 +then + die "unable to patch musl NIF dependencies" +fi printf 'prepared musl NIF: %s\n' "${mdex_nifs[0]}" printf 'prepared musl NIF: %s\n' "$syntect_host_nif" diff --git a/ci/test_burrito_shared_loader.sh b/ci/test_burrito_shared_loader.sh index 47bc021..5bab647 100755 --- a/ci/test_burrito_shared_loader.sh +++ b/ci/test_burrito_shared_loader.sh @@ -1,20 +1,36 @@ #!/usr/bin/env bash -set -euo pipefail +die() { + printf 'ERROR: %s\n' "$*" >&2 + exit 1 +} -if [ "$#" -ne 1 ]; then +must() { + if "$@" + then + return 0 + fi + + die "command failed: $*" +} + +if [ "$#" -ne 1 ] +then printf 'usage: %s BURRITO_BINARY\n' "$0" >&2 exit 64 fi -binary=$(realpath "$1") +binary=$(realpath "$1") || die "unable to resolve Burrito binary: $1" -if [ ! -x "$binary" ]; then +if [ ! -x "$binary" ] +then printf 'Burrito binary is not executable: %s\n' "$binary" >&2 exit 1 fi -case "$(uname -m)" in +architecture=$(uname -m) || die "unable to determine architecture" + +case "$architecture" in x86_64) runtime_hash=71c35316aff45bbfd243d8eb9bfc4a58b6eb97cee09514cd2030e145b68107fb ;; @@ -22,15 +38,18 @@ case "$(uname -m)" in runtime_hash=6b558025200a5ed1308e2ce2675217afec71b6c5a9d561e52262ca948d59905e ;; *) - printf 'Unsupported Linux architecture: %s\n' "$(uname -m)" >&2 + printf 'Unsupported Linux architecture: %s\n' "$architecture" >&2 exit 1 ;; esac -sudo -n true +if ! sudo -n true +then + die "passwordless sudo is required for the shared-loader regression" +fi -test_root=$(mktemp -d /tmp/lc-shared-loader-test.XXXXXX) -chmod 0711 "$test_root" +test_root=$(mktemp -d /tmp/lc-shared-loader-test.XXXXXX) || die "unable to create temporary test directory" +must chmod 0711 "$test_root" user_a="lcx17a$$" user_b="lcx17b$$" legacy_loader="/tmp/libc-musl-${runtime_hash}.so" @@ -38,43 +57,68 @@ legacy_backup="" private_runtime_dirs=() cleanup() { - sudo rm -f -- "$legacy_loader" + if ! sudo rm -f -- "$legacy_loader" + then + printf 'WARNING: cleanup could not remove legacy loader: %s\n' "$legacy_loader" >&2 + fi - for runtime_dir in "${private_runtime_dirs[@]}"; do - sudo rm -rf -- "$runtime_dir" + for runtime_dir in "${private_runtime_dirs[@]}" + do + if ! sudo rm -rf -- "$runtime_dir" + then + printf 'WARNING: cleanup could not remove private runtime: %s\n' "$runtime_dir" >&2 + fi done - if [ -n "$legacy_backup" ] && sudo test -e "$legacy_backup"; then - sudo mv -- "$legacy_backup" "$legacy_loader" + if [ -n "$legacy_backup" ] && sudo test -e "$legacy_backup" + then + if ! sudo mv -- "$legacy_backup" "$legacy_loader" + then + printf 'WARNING: cleanup could not restore legacy loader: %s\n' "$legacy_loader" >&2 + fi fi - sudo userdel "$user_a" 2>/dev/null || true - sudo userdel "$user_b" 2>/dev/null || true - sudo rm -rf -- "$test_root" + if ! sudo userdel "$user_a" 2>/dev/null + then + : + fi + + if ! sudo userdel "$user_b" 2>/dev/null + then + : + fi + + if ! sudo rm -rf -- "$test_root" + then + printf 'WARNING: cleanup could not remove test directory: %s\n' "$test_root" >&2 + fi } trap cleanup EXIT -if sudo test -e "$legacy_loader"; then +if sudo test -e "$legacy_loader" +then legacy_backup="$test_root/original-legacy-loader" - sudo mv -- "$legacy_loader" "$legacy_backup" + must sudo mv -- "$legacy_loader" "$legacy_backup" fi binary_copy="$test_root/lc" -sudo install -m 0755 -- "$binary" "$binary_copy" +must sudo install -m 0755 -- "$binary" "$binary_copy" -for user in "$user_a" "$user_b"; do +for user in "$user_a" "$user_b" +do user_dir="$test_root/$user" - sudo mkdir -- "$user_dir" - sudo useradd --no-create-home --home-dir "$user_dir" --shell /bin/bash "$user" - sudo chown "$user:$user" "$user_dir" - private_runtime_dirs+=("/tmp/.burrito-musl-$(id -u "$user")") + must sudo mkdir -- "$user_dir" + must sudo useradd --no-create-home --home-dir "$user_dir" --shell /bin/bash "$user" + must sudo chown "$user:$user" "$user_dir" + user_uid=$(id -u "$user") || die "unable to determine UID for $user" + private_runtime_dirs+=("/tmp/.burrito-musl-$user_uid") done # Recreate the affected-release state: user A owns a predictable shared # loader at 0754, and the bytes are explicitly not Burrito's embedded loader. -sudo -u "$user_a" sh -c 'printf %s untrusted-prepositioned-loader > "$1"' sh "$legacy_loader" -sudo -u "$user_a" chmod 0754 "$legacy_loader" +must sudo -u "$user_a" sh -c 'printf %s untrusted-prepositioned-loader > "$1"' sh "$legacy_loader" +must sudo -u "$user_a" chmod 0754 "$legacy_loader" run_version() { local user=$1 @@ -93,8 +137,8 @@ find_erts_binary() { } interpreter_for() { - sudo readelf -l "$1" | - sed -n 's/.*Requesting program interpreter: \(.*\)]/\1/p' + program_headers=$(sudo readelf -l "$1") || return 1 + printf '%s\n' "$program_headers" | sed -n 's/.*Requesting program interpreter: \(.*\)]/\1/p' } private_runtime_for() { @@ -110,7 +154,8 @@ assert_equal() { local expected=$2 local description=$3 - if [ "$actual" != "$expected" ]; then + if [ "$actual" != "$expected" ] + then printf '%s mismatch\nexpected: %s\nactual: %s\n' \ "$description" "$expected" "$actual" >&2 return 1 @@ -133,7 +178,8 @@ assert_private_runtime() { erlexec=$(find_erts_binary "$user" erlexec) || return 1 beam=$(find_erts_binary "$user" beam.smp) || return 1 - if [ -z "$erlexec" ] || [ -z "$beam" ]; then + if [ -z "$erlexec" ] || [ -z "$beam" ] + then printf 'Could not find both ERTS executables for %s\n' "$user" >&2 return 1 fi @@ -142,7 +188,8 @@ assert_private_runtime() { beam_interpreter=$(interpreter_for "$beam") || return 1 runtime_dir_metadata=$(sudo stat -c '%u:%a:%F' "$(dirname "$expected_interpreter")") || return 1 loader_metadata=$(sudo stat -c '%u:%a:%F' "$expected_interpreter") || return 1 - loader_hash=$(sudo sha256sum "$expected_interpreter" | cut -d ' ' -f 1) || return 1 + loader_checksum=$(sudo sha256sum "$expected_interpreter") || return 1 + loader_hash=${loader_checksum%% *} assert_equal "$erlexec_interpreter" "$expected_interpreter" "erlexec interpreter" || return 1 assert_equal "$beam_interpreter" "$expected_interpreter" "beam.smp interpreter" || return 1 @@ -151,105 +198,121 @@ assert_private_runtime() { assert_equal "$loader_hash" "$runtime_hash" "private loader hash" || return 1 } -run_version "$user_a" -runtime_a=$(private_runtime_for "$user_a") -assert_private_runtime "$user_a" "$runtime_a" +must run_version "$user_a" +runtime_a=$(private_runtime_for "$user_a") || die "unable to determine private runtime for $user_a" +must assert_private_runtime "$user_a" "$runtime_a" # The UID-scoped path is short enough to fit the existing ELF interpreter # segment, so its name is deterministic. An attacker may pre-position it, but # ownership validation must fail closed without executing the attacker's file. -uid_b=$(id -u "$user_b") -attacker_runtime=$(private_runtime_for "$user_b") -attacker_runtime_dir=$(dirname "$attacker_runtime") -sudo -u "$user_a" mkdir -m 0755 -- "$attacker_runtime_dir" -sudo -u "$user_a" sh -c 'printf %s untrusted-private-loader > "$1"' sh "$attacker_runtime" - -if run_version "$user_b" >"$test_root/prepositioned-private.log" 2>&1; then +uid_b=$(id -u "$user_b") || die "unable to determine UID for $user_b" +attacker_runtime=$(private_runtime_for "$user_b") || die "unable to determine private runtime for $user_b" +attacker_runtime_dir=$(dirname "$attacker_runtime") || die "unable to determine attacker runtime directory" +must sudo -u "$user_a" mkdir -m 0755 -- "$attacker_runtime_dir" +must sudo -u "$user_a" sh -c 'printf %s untrusted-private-loader > "$1"' sh "$attacker_runtime" + +if run_version "$user_b" >"$test_root/prepositioned-private.log" 2>&1 +then printf 'Burrito trusted an attacker-owned private runtime path\n' >&2 exit 1 fi -grep -Fq UntrustedMuslRuntime "$test_root/prepositioned-private.log" -sudo rm -rf -- "$attacker_runtime_dir" +if ! grep -Fq UntrustedMuslRuntime "$test_root/prepositioned-private.log" +then + die "Burrito did not report the untrusted private runtime" +fi +must sudo rm -rf -- "$attacker_runtime_dir" -run_version "$user_b" -runtime_b=$(private_runtime_for "$user_b") -assert_private_runtime "$user_b" "$runtime_b" +must run_version "$user_b" +runtime_b=$(private_runtime_for "$user_b") || die "unable to determine private runtime for $user_b" +must assert_private_runtime "$user_b" "$runtime_b" # Prove the assertion helper itself cannot silently succeed after a failed # check, including when called from an `if` condition where errexit is disabled. -sudo chmod 0701 "$runtime_b" -if assert_private_runtime "$user_b" "$runtime_b" >"$test_root/assertion-negative.log" 2>&1; then +must sudo chmod 0701 "$runtime_b" +if assert_private_runtime "$user_b" "$runtime_b" >"$test_root/assertion-negative.log" 2>&1 +then printf 'Private-runtime assertions accepted an invalid loader mode\n' >&2 exit 1 fi -grep -Fq 'private loader metadata mismatch' "$test_root/assertion-negative.log" -sudo chmod 0700 "$runtime_b" -assert_private_runtime "$user_b" "$runtime_b" +if ! grep -Fq 'private loader metadata mismatch' "$test_root/assertion-negative.log" +then + die "private-runtime assertion did not report the invalid loader mode" +fi +must sudo chmod 0700 "$runtime_b" +must assert_private_runtime "$user_b" "$runtime_b" -if [ "$runtime_a" = "$runtime_b" ]; then +if [ "$runtime_a" = "$runtime_b" ] +then printf 'Both users selected the same private runtime: %s\n' "$runtime_a" >&2 exit 1 fi -legacy_contents=$(sudo cat "$legacy_loader") -legacy_metadata=$(sudo stat -c '%U:%a' "$legacy_loader") -assert_equal "$legacy_contents" untrusted-prepositioned-loader "legacy loader contents" -assert_equal "$legacy_metadata" "$user_a:754" "legacy loader metadata" +legacy_contents=$(sudo cat "$legacy_loader") || die "unable to read legacy loader" +legacy_metadata=$(sudo stat -c '%U:%a' "$legacy_loader") || die "unable to inspect legacy loader metadata" +must assert_equal "$legacy_contents" untrusted-prepositioned-loader "legacy loader contents" +must assert_equal "$legacy_metadata" "$user_a:754" "legacy loader metadata" # A valid version/hash/path marker avoids reopening the full extracted release # on every warm launch. An unreadable unrelated file proves the fast path is # used; corrupting the marker must force a rescan and expose that read error. -erlexec_b=$(find_erts_binary "$user_b" erlexec) -erts_bin_dir=$(dirname "$erlexec_b") -erts_dir=$(dirname "$erts_bin_dir") -install_dir_b=$(dirname "$erts_dir") +erlexec_b=$(find_erts_binary "$user_b" erlexec) || die "unable to find erlexec for $user_b" +erts_bin_dir=$(dirname "$erlexec_b") || die "unable to resolve ERTS binary directory" +erts_dir=$(dirname "$erts_bin_dir") || die "unable to resolve ERTS directory" +install_dir_b=$(dirname "$erts_dir") || die "unable to resolve Burrito installation directory" marker_b="$install_dir_b/.burrito-musl-interpreters-v1" -marker_metadata=$(sudo stat -c '%u:%a:%F' "$marker_b") -marker_contents=$(sudo cat "$marker_b") +marker_metadata=$(sudo stat -c '%u:%a:%F' "$marker_b") || die "unable to inspect interpreter marker metadata" +marker_contents=$(sudo cat "$marker_b") || die "unable to read interpreter marker" expected_marker=$(printf 'v1\n%s\n%s\n' "$runtime_hash" "$runtime_b") -assert_equal "$marker_metadata" "$uid_b:600:regular file" "interpreter marker metadata" -assert_equal "$marker_contents" "$expected_marker" "interpreter marker contents" +must assert_equal "$marker_metadata" "$uid_b:600:regular file" "interpreter marker metadata" +must assert_equal "$marker_contents" "$expected_marker" "interpreter marker contents" walk_sentinel="$install_dir_b/unreadable-walk-sentinel" -sudo -u "$user_b" touch "$walk_sentinel" -sudo -u "$user_b" chmod 000 "$walk_sentinel" -run_version "$user_b" +must sudo -u "$user_b" touch "$walk_sentinel" +must sudo -u "$user_b" chmod 000 "$walk_sentinel" +must run_version "$user_b" -sudo -u "$user_b" sh -c 'printf %s corrupt-marker > "$1"' sh "$marker_b" -sudo -u "$user_b" chmod 0600 "$marker_b" +must sudo -u "$user_b" sh -c 'printf %s corrupt-marker > "$1"' sh "$marker_b" +must sudo -u "$user_b" chmod 0600 "$marker_b" -if run_version "$user_b" >"$test_root/invalid-marker.log" 2>&1; then +if run_version "$user_b" >"$test_root/invalid-marker.log" 2>&1 +then printf 'Burrito trusted an invalid interpreter marker\n' >&2 exit 1 fi -grep -Fq AccessDenied "$test_root/invalid-marker.log" +if ! grep -Fq AccessDenied "$test_root/invalid-marker.log" +then + die "Burrito did not report the corrupt interpreter marker" +fi -sudo rm -f -- "$walk_sentinel" -run_version "$user_b" -marker_contents=$(sudo cat "$marker_b") -assert_equal "$marker_contents" "$expected_marker" "repaired interpreter marker contents" -assert_private_runtime "$user_b" "$runtime_b" +must sudo rm -f -- "$walk_sentinel" +must run_version "$user_b" +marker_contents=$(sudo cat "$marker_b") || die "unable to read repaired interpreter marker" +must assert_equal "$marker_contents" "$expected_marker" "repaired interpreter marker contents" +must assert_private_runtime "$user_b" "$runtime_b" # Replace the hostile object with the real loader bytes in the state left by # an affected release: user A owns a valid shared loader at 0754. Reinstalling # user B's payload must still ignore that object and choose user B's directory. -sudo rm -f -- "$legacy_loader" -sudo -u "$user_a" cp -- "$runtime_a" "$legacy_loader" -sudo -u "$user_a" chmod 0754 "$legacy_loader" -sudo rm -rf -- "$test_root/$user_b/data" "$(dirname "$runtime_b")" -run_version "$user_b" -runtime_b=$(private_runtime_for "$user_b") -assert_private_runtime "$user_b" "$runtime_b" -legacy_hash=$(sudo sha256sum "$legacy_loader" | cut -d ' ' -f 1) -legacy_metadata=$(sudo stat -c '%U:%a' "$legacy_loader") -assert_equal "$legacy_hash" "$runtime_hash" "stale legacy loader hash" -assert_equal "$legacy_metadata" "$user_a:754" "stale legacy loader metadata" +must sudo rm -f -- "$legacy_loader" +must sudo -u "$user_a" cp -- "$runtime_a" "$legacy_loader" +must sudo -u "$user_a" chmod 0754 "$legacy_loader" +runtime_b_dir=$(dirname "$runtime_b") || die "unable to determine user B private runtime directory" +must sudo rm -rf -- "$test_root/$user_b/data" "$runtime_b_dir" +must run_version "$user_b" +runtime_b=$(private_runtime_for "$user_b") || die "unable to determine private runtime for $user_b" +must assert_private_runtime "$user_b" "$runtime_b" +legacy_checksum=$(sudo sha256sum "$legacy_loader") || die "unable to calculate legacy loader checksum" +legacy_hash=${legacy_checksum%% *} +legacy_metadata=$(sudo stat -c '%U:%a' "$legacy_loader") || die "unable to inspect legacy loader metadata" +must assert_equal "$legacy_hash" "$runtime_hash" "stale legacy loader hash" +must assert_equal "$legacy_metadata" "$user_a:754" "stale legacy loader metadata" # `/tmp` can be cleared while Burrito's extracted release remains. The next # launch must recreate and revalidate the private loader without re-extracting. -sudo rm -rf -- "$(dirname "$runtime_b")" -run_version "$user_b" -assert_private_runtime "$user_b" "$runtime_b" +runtime_b_dir=$(dirname "$runtime_b") || die "unable to determine user B private runtime directory" +must sudo rm -rf -- "$runtime_b_dir" +must run_version "$user_b" +must assert_private_runtime "$user_b" "$runtime_b" printf 'Burrito shared-loader regression passed for %s and %s\n' "$user_a" "$user_b" diff --git a/ci/validate_commit_range.sh b/ci/validate_commit_range.sh index 9c862c5..e7b8e03 100755 --- a/ci/validate_commit_range.sh +++ b/ci/validate_commit_range.sh @@ -105,8 +105,32 @@ base_sha=$(git_or_die merge-base HEAD "$base_ref") validation_status=0 -while IFS= read -r -d '' subject +# GitHub's "Update branch" button creates a non-Conventional-Commit merge +# subject. Exempt only its trusted, canonical form; ordinary contributor merge +# commits must still pass subject validation. See +# documents/github-update-branch-validation-decision.adoc. +github_update_branch_merge_pattern="^Merge branch '[^']+' into .+" + +while IFS= read -r -d '' entry do + IFS=$'\x01' read -r parents committer_name committer_email subject <<< "$entry" + + if [ -n "$parents" ] + then + IFS=' ' read -ra parents_array <<< "$parents" + parent_count=${#parents_array[@]} + else + parent_count=0 + fi + + if [ "$parent_count" -eq 2 ] \ + && [ "$committer_name" = "GitHub" ] \ + && [ "$committer_email" = "noreply@github.com" ] \ + && [[ "$subject" =~ $github_update_branch_merge_pattern ]] + then + continue + fi + "$validator" --subject "$subject" status=$? @@ -114,6 +138,6 @@ do then validation_status=$status fi -done < <(git log -z --format='%s' "$base_sha..HEAD") +done < <(git log -z --format='%P%x01%cn%x01%ce%x01%s' "$base_sha..HEAD") exit "$validation_status" diff --git a/documents/quality-gates-decision.adoc b/documents/quality-gates-decision.adoc index 4ad464c..8eec0c6 100644 --- a/documents/quality-gates-decision.adoc +++ b/documents/quality-gates-decision.adoc @@ -78,8 +78,15 @@ than relying on an undocumented alias. == Current implementation -At the time of this decision, `mix ci` delegates directly to `mix precommit`, -and `mix precommit` performs dependency retrieval, dependency audits, and the -full application suite. That arrangement does not satisfy this decision. -Implementation work must split the tasks so `mix precommit` has the local -contract above and `mix ci` composes it with the CI-only work. +`mix precommit` runs only source-local checks (root and app formatting, +static analysis, and unit tests). It requires no network access and no +installed services beyond the language toolchain and already-fetched +dependencies. + +`mix ci` is an independent orchestrator. It installs dependencies, runs fast +metadata guards (PR-title and commit-range validation), invokes `mix precommit` +in full, and then performs the CI-only checks (Hex and dependency security +audits and usage-rule drift), followed by the unfiltered app test suite. That +final test run includes both ordinary tests and any future tests tagged +`ci_only`; the tag is excluded only by `mix precommit`, never globally. GitHub +Actions calls `mix ci`, not `mix precommit`. diff --git a/documents/style/bash.adoc b/documents/style/bash.adoc new file mode 100644 index 0000000..ab903bb --- /dev/null +++ b/documents/style/bash.adoc @@ -0,0 +1,66 @@ += Bash Style + +== Error handling + +Never use `set -e`, `set -u`, or `set -o pipefail`, including combined forms +such as `set -euo pipefail`. Their context-sensitive behavior makes failures +hard to reason about. + +Check every command that can fail explicitly, with an actionable diagnostic and +an intentional exit or return path: + +[source,bash] +---- +if ! install -m 0755 "$source" "$destination" +then + printf 'ERROR: unable to install %s\n' "$destination" >&2 + exit 1 +fi +---- + +For a command substitution, check the assignment itself: + +[source,bash] +---- +checksum=$(sha256sum "$artifact") || { + printf 'ERROR: unable to calculate checksum for %s\n' "$artifact" >&2 + exit 1 +} +---- + +Use a small helper only when it retains a clear diagnostic at every call site; +do not capture `$?` after an unchecked command. Commands expected to fail as +part of a test must be placed in an explicit `if` branch and have both outcomes +handled. + +Cleanup is best-effort, but it is still explicit: report a warning for a failed +cleanup action and continue cleaning the remaining resources. + +== Control flow and pipelines + +Put `then` and `do` on their own lines. Do not write `; then` or `; do`. + +[source,bash] +---- +if [ -f "$file" ] +then + process_file "$file" +fi + +for file in "${files[@]}" +do + process_file "$file" +done +---- + +Avoid pipelines when an intermediate command's failure matters. Capture the +output of the fallible command first, check it, and then process the captured +value. If a pipeline is unavoidable, put it in an explicit conditional and +ensure every producer failure is independently checked before the pipeline. + +== Shell scope + +Prefer Bash only when Bash features are required, and declare it with +`#!/usr/bin/env bash`. Quote expansions unless deliberate word splitting or +pathname expansion is required. Keep hook adapters small; put behavior in +directly testable `ci/*.sh` programs. diff --git a/git-hooks/pre-push b/git-hooks/pre-push index 3eba12c..2dcaf4e 100755 --- a/git-hooks/pre-push +++ b/git-hooks/pre-push @@ -1,7 +1,8 @@ #!/bin/sh -# Validates every commit subject introduced by this push. -# See app/usage-rules.md for the rule. Activate with: +# Validates every commit subject introduced by this push, then rejects known +# vulnerable or retired Hex dependencies. See app/usage-rules.md. Activate with: # mix git_hooks repo_top=$(git rev-parse --show-toplevel) || exit 1 -exec "$repo_top/ci/validate_push_refs.sh" "$@" +"$repo_top/ci/validate_push_refs.sh" "$@" || exit $? +exec "$repo_top/ci/hex-audit.sh" diff --git a/lib/mix/tasks/ci.ex b/lib/mix/tasks/ci.ex index 3ddc75f..9cd2518 100644 --- a/lib/mix/tasks/ci.ex +++ b/lib/mix/tasks/ci.ex @@ -1,27 +1,69 @@ defmodule Mix.Tasks.Ci do - @shortdoc "Compatibility alias for mix precommit" + @shortdoc "Complete integration gate: local checks plus CI-only audits and validation" @moduledoc """ #{@shortdoc}. mix ci - Delegates to `Mix.Tasks.Precommit`, which is the canonical full-repository - quality gate. Kept for backwards compatibility with scripts and CI - configurations that call `mix ci` directly. + The canonical pull-request and merge gate, and the command GitHub Actions + invokes. It bootstraps dependencies, runs fast metadata guards, runs the full + local gate, then performs CI-only checks that may use the network, PR + metadata, or advisory services. - See `mix help precommit` for the complete step list. + Steps: + + 1. `mix deps.get` — ensure app dependencies are installed + 2. `ci/validate_pull_request_title.sh` — require a Conventional Commits PR + title when `PULL_REQUEST_TITLE_REQUIRED=true` + 3. `ci/validate_commit_range.sh` — validate every commit since the branch + diverged from its base + 4. `mix precommit` — the fast local gate (format, static analysis, unit tests) + 5. `mix hex.audit` — reject retired or vulnerable Hex packages + 6. `mix deps.audit` — scan dependencies for known security advisories + 7. `mix usage_rules.sync --check` — catch usage-rule drift after dep bumps + 8. `mix test` — run the complete app test suite, including CI-classified tests + + Step 1 and steps 5-8 run inside `app/`. Steps 2-3 run from the repo root. + Step 4 expands to all of `mix precommit`'s steps in place. + + Every check in `mix precommit` also runs through `mix ci`. The relationship + is: + + mix precommit ⊂ mix ci + + Pull request metadata does not exist before a pull request is opened, so + local runs of `mix ci` skip only the title guard. GitHub Actions sets both + `PULL_REQUEST_TITLE_REQUIRED=true` and `PULL_REQUEST_TITLE` from the event; + a missing, empty, or non-conventional title then fails step 2. Commit subjects + are always validated in step 3. + + For the fast local gate only (no network, no CI metadata), use `mix precommit`. """ use Mix.Task + alias RepoTasks.Shell + @impl Mix.Task def run(argv) do - Mix.Tasks.Precommit.run(argv) + run(argv, &Shell.run!/3) end @doc false - def run(argv, shell) do - Mix.Tasks.Precommit.run(argv, shell) + def run([], shell) do + shell.("mix", ["deps.get"], cd: "app") + shell.("./ci/validate_pull_request_title.sh", [], []) + shell.("./ci/validate_commit_range.sh", [], []) + Mix.Tasks.Precommit.run([], shell) + shell.("mix", ["hex.audit"], cd: "app") + shell.("mix", ["deps.audit"], cd: "app") + shell.("mix", ["usage_rules.sync", "--check"], cd: "app") + shell.("mix", ["test"], cd: "app") + :ok + end + + def run(_argv, _shell) do + Mix.raise("Usage: mix ci") end end diff --git a/lib/mix/tasks/git_hooks.ex b/lib/mix/tasks/git_hooks.ex index 16daa77..ffe0545 100644 --- a/lib/mix/tasks/git_hooks.ex +++ b/lib/mix/tasks/git_hooks.ex @@ -1,5 +1,5 @@ defmodule Mix.Tasks.GitHooks do - @shortdoc "Installs this repo's commit-msg and pre-push validation hooks" + @shortdoc "Installs this repo's commit-msg and pre-push quality hooks" @moduledoc """ #{@shortdoc}. @@ -7,7 +7,9 @@ defmodule Mix.Tasks.GitHooks do mix git_hooks Sets `core.hooksPath` to `git-hooks/`: its `commit-msg` hook enforces - Conventional Commits, and its `pre-push` hook runs `mix precommit`. + Conventional Commits on each commit subject, and its `pre-push` hook + validates all commit subjects introduced by the push before running Hex's + dependency security audit. This is idempotent and safe to run repeatedly: setting the same Git config value twice is a no-op. Wired into `mix setup` - see that task. """ diff --git a/lib/mix/tasks/precommit.ex b/lib/mix/tasks/precommit.ex index c871301..18b8418 100644 --- a/lib/mix/tasks/precommit.ex +++ b/lib/mix/tasks/precommit.ex @@ -1,36 +1,33 @@ defmodule Mix.Tasks.Precommit do - @shortdoc "Runs every local and CI validation for this repository" + @shortdoc "Fast local quality gate (format, static analysis, unit tests)" @moduledoc """ #{@shortdoc}. mix precommit - This is the single validation entrypoint for developers, Git hooks, and - GitHub Actions. Cheap metadata guards run first so an invalid pull request - title or commit subject fails before dependency setup and the test suite: + A fast, self-contained command designed for frequent developer use: between + edits, before committing, and as the pre-push hook target. On a warm checkout + with dependencies already installed, it completes in under five seconds. - 1. `ci/validate_pull_request_title.sh` — require a Conventional Commits - pull request title when `PULL_REQUEST_TITLE_REQUIRED=true` - 2. `ci/validate_commit_range.sh` — validate every commit since the branch - diverged from its base - 3. `mix format --check-formatted` — check root project formatting - 4. `mix test` — run the root project test suite (validator and task tests) - 5. `mix deps.get` — ensure app dependencies are present - 6. `mix hex.audit` — reject retired or vulnerable Hex packages - 7. `mix deps.audit` — scan dependencies for known security advisories - 8. `mix format --check-formatted` — check app formatting - 9. `mix credo --strict` — run static analysis - 10. `mix usage_rules.sync --check` — catch usage-rule drift after dep bumps - 11. `mix test` — run the app test suite + It requires no network access, credentials, containers, or external services. + Run `mix deps.get` inside `app/` once after cloning or after updating + `app/mix.lock`, then run `mix precommit` as often as you like. - Pull request metadata does not exist before a pull request is opened, so - local runs skip only the title guard. GitHub Actions sets both - `PULL_REQUEST_TITLE_REQUIRED=true` and `PULL_REQUEST_TITLE` from the event; - a missing, empty, or non-conventional title then fails this task. Commit - subjects are always validated. + Steps: - Steps 1-4 run from the repo root; steps 5-11 run inside `app/`. + 1. `mix format --check-formatted` — check root project formatting + 2. `mix test` — run the root project test suite (validator and task tests) + 3. `mix format --check-formatted` — check app formatting + 4. `mix credo --strict` — run static analysis on the app + 5. `mix test --exclude ci_only` — run the app unit test suite + + Steps 1-2 run from the repo root; steps 3-5 run inside `app/`. + App tests tagged `@moduletag :ci_only` are excluded only from this local + gate. `mix ci` runs the complete app suite without a test-selection flag. + + For the complete integration gate — dependency bootstrap and audits, + PR-title and commit-range validation, and CI-classified tests — use `mix ci`. """ use Mix.Task @@ -44,17 +41,11 @@ defmodule Mix.Tasks.Precommit do @doc false def run([], shell) do - shell.("./ci/validate_pull_request_title.sh", [], []) - shell.("./ci/validate_commit_range.sh", [], []) shell.("mix", ["format", "--check-formatted"], []) shell.("mix", ["test"], []) - shell.("mix", ["deps.get"], cd: "app") - shell.("mix", ["hex.audit"], cd: "app") - shell.("mix", ["deps.audit"], cd: "app") shell.("mix", ["format", "--check-formatted"], cd: "app") shell.("mix", ["credo", "--strict"], cd: "app") - shell.("mix", ["usage_rules.sync", "--check"], cd: "app") - shell.("mix", ["test"], cd: "app") + shell.("mix", ["test", "--exclude", "ci_only"], cd: "app") :ok end diff --git a/test/git_hooks_test.exs b/test/git_hooks_test.exs index 801c318..676f5be 100644 --- a/test/git_hooks_test.exs +++ b/test/git_hooks_test.exs @@ -145,12 +145,11 @@ defmodule GitHooksTest do refute output =~ "feat: valid commit" end - test "the range guard validates a GitHub Update-branch merge commit" do + test "the range guard skips a GitHub Update-branch merge commit matching all three predicates" do {worktree, _} = setup_ci_worktree!() add_github_merge!(worktree) - assert {output, 1} = run(@range_guard, [], cd: worktree) - assert output =~ "Merge branch 'main' into feature" + assert {"", 0} = run(@range_guard, [], cd: worktree) end test "the range guard validates when the committer name is not GitHub" do @@ -253,6 +252,41 @@ defmodule GitHooksTest do assert entries == ["commit-msg", "pre-push"] end + test "the pre-push adapter validates refs before running the Hex audit" do + {worktree, ci_dir} = setup_ci_worktree!() + audit_marker = Path.join(worktree, "hex-audit-ran") + hook = install_pre_push_hook!(worktree, ci_dir) + + {base_sha, 0} = System.cmd("git", ["rev-parse", "HEAD"], cd: worktree) + base_sha = String.trim(base_sha) + valid_stdin = "refs/heads/main #{base_sha} refs/heads/main #{base_sha}\n" + + assert {"", 0} = + run_with_stdin(hook, valid_stdin, + cd: worktree, + env: [{"HEX_AUDIT_MARKER", audit_marker}] + ) + + assert File.read!(audit_marker) == "audited\n" + File.rm!(audit_marker) + + File.write!(Path.join(worktree, "invalid"), "commit\n") + git!(worktree, ["add", "invalid"]) + git!(worktree, ["commit", "-m", "not conventional"]) + {head_sha, 0} = System.cmd("git", ["rev-parse", "HEAD"], cd: worktree) + head_sha = String.trim(head_sha) + invalid_stdin = "refs/heads/main #{head_sha} refs/heads/main #{base_sha}\n" + + assert {output, 1} = + run_with_stdin(hook, invalid_stdin, + cd: worktree, + env: [{"HEX_AUDIT_MARKER", audit_marker}] + ) + + assert output =~ "not conventional" + refute File.exists?(audit_marker) + end + # Creates a no-fast-forward merge commit on `main` from a throwaway `feature` # branch. Defaults simulate GitHub's "Update branch" identity and subject; # these remain subject to validation because commit metadata is forgeable. @@ -318,6 +352,21 @@ defmodule GitHooksTest do {worktree, ci_dir} end + defp install_pre_push_hook!(worktree, ci_dir) do + hooks_dir = Path.join(worktree, "git-hooks") + File.mkdir_p!(hooks_dir) + + hook = Path.join(hooks_dir, "pre-push") + File.cp!(Path.expand("../git-hooks/pre-push", __DIR__), hook) + File.chmod!(hook, 0o755) + + audit = Path.join(ci_dir, "hex-audit.sh") + File.write!(audit, "#!/bin/sh\nprintf 'audited\\n' > \"$HEX_AUDIT_MARKER\"\n") + File.chmod!(audit, 0o755) + + hook + end + defp run_with_stdin(command, stdin_content, opts) do nonce = :crypto.strong_rand_bytes(8) |> Base.url_encode64(padding: false) stdin_file = Path.join(System.tmp_dir!(), "linear_cli_stdin_#{nonce}") diff --git a/test/mix/tasks/ci_test.exs b/test/mix/tasks/ci_test.exs index 78f689f..b75f0ec 100644 --- a/test/mix/tasks/ci_test.exs +++ b/test/mix/tasks/ci_test.exs @@ -3,7 +3,7 @@ defmodule Mix.Tasks.CiTest do alias Mix.Tasks.Ci - test "delegates to mix precommit" do + test "bootstraps deps, validates, runs precommit checks, then CI-only checks" do caller = self() shell = fn cmd, args, opts -> @@ -13,21 +13,27 @@ defmodule Mix.Tasks.CiTest do assert :ok = Ci.run([], shell) + # Bootstrap + assert_received {:run, "mix", ["deps.get"], [cd: "app"]} + # Metadata guards (fast failures before the expensive steps) assert_received {:run, "./ci/validate_pull_request_title.sh", [], []} assert_received {:run, "./ci/validate_commit_range.sh", [], []} + # All precommit local checks are present (mix precommit ⊂ mix ci) assert_received {:run, "mix", ["format", "--check-formatted"], []} assert_received {:run, "mix", ["test"], []} - assert_received {:run, "mix", ["deps.get"], [cd: "app"]} - assert_received {:run, "mix", ["hex.audit"], [cd: "app"]} - assert_received {:run, "mix", ["deps.audit"], [cd: "app"]} assert_received {:run, "mix", ["format", "--check-formatted"], [cd: "app"]} assert_received {:run, "mix", ["credo", "--strict"], [cd: "app"]} + assert_received {:run, "mix", ["test", "--exclude", "ci_only"], [cd: "app"]} + # CI-only audits, followed by the unfiltered app suite. + assert_received {:run, "mix", ["hex.audit"], [cd: "app"]} + assert_received {:run, "mix", ["deps.audit"], [cd: "app"]} assert_received {:run, "mix", ["usage_rules.sync", "--check"], [cd: "app"]} assert_received {:run, "mix", ["test"], [cd: "app"]} + refute_received {:run, "mix", ["test", "--only", "ci_only"], _} end - test "rejects arguments via precommit" do - assert_raise Mix.Error, "Usage: mix precommit", fn -> + test "rejects arguments" do + assert_raise Mix.Error, "Usage: mix ci", fn -> Ci.run(["unexpected"], fn _, _, _ -> :ok end) end end diff --git a/test/mix/tasks/precommit_test.exs b/test/mix/tasks/precommit_test.exs index 3af847d..2b06c05 100644 --- a/test/mix/tasks/precommit_test.exs +++ b/test/mix/tasks/precommit_test.exs @@ -3,7 +3,7 @@ defmodule Mix.Tasks.PrecommitTest do alias Mix.Tasks.Precommit - test "runs metadata guards before all quality gate steps" do + test "runs only local checks in order" do caller = self() shell = fn cmd, args, opts -> @@ -13,17 +13,36 @@ defmodule Mix.Tasks.PrecommitTest do assert :ok = Precommit.run([], shell) - assert_received {:run, "./ci/validate_pull_request_title.sh", [], []} - assert_received {:run, "./ci/validate_commit_range.sh", [], []} assert_received {:run, "mix", ["format", "--check-formatted"], []} assert_received {:run, "mix", ["test"], []} - assert_received {:run, "mix", ["deps.get"], [cd: "app"]} - assert_received {:run, "mix", ["hex.audit"], [cd: "app"]} - assert_received {:run, "mix", ["deps.audit"], [cd: "app"]} assert_received {:run, "mix", ["format", "--check-formatted"], [cd: "app"]} assert_received {:run, "mix", ["credo", "--strict"], [cd: "app"]} - assert_received {:run, "mix", ["usage_rules.sync", "--check"], [cd: "app"]} - assert_received {:run, "mix", ["test"], [cd: "app"]} + assert_received {:run, "mix", ["test", "--exclude", "ci_only"], [cd: "app"]} + end + + test "does not run CI-only steps" do + caller = self() + + shell = fn cmd, args, opts -> + send(caller, {:run, cmd, args, opts}) + :ok + end + + Precommit.run([], shell) + + refute_received {:run, "./ci/validate_pull_request_title.sh", _, _} + refute_received {:run, "./ci/validate_commit_range.sh", _, _} + refute_received {:run, "mix", ["deps.get"], _} + refute_received {:run, "mix", ["hex.audit"], _} + refute_received {:run, "mix", ["deps.audit"], _} + refute_received {:run, "mix", ["usage_rules.sync", "--check"], _} + refute_received {:run, "mix", ["test", "--only", "ci_only"], _} + end + + test "does not globally exclude CI-only tests" do + helper = Path.expand("../../../app/test/test_helper.exs", __DIR__) + + refute File.read!(helper) =~ "ExUnit.start(exclude: [:ci_only])" end test "rejects arguments" do