From 163caf78e63d901dbfe6f29be4d2b2f188d3639a Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:52:26 -0500 Subject: [PATCH 01/25] docs: fix GitHub App credential lifecycle guidance --- docs/GITHUB-APP-SETUP.md | 148 ++++++++++++++++++++++++++++++++----- scripts/test_quickstart.py | 18 ++++- 2 files changed, 144 insertions(+), 22 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 7ada9610..f521da50 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -44,11 +44,41 @@ Nothing else. No `write` on contents, no actions, no administration. If the controller ever needs more, that is a reviewed design change, not a settings tweak. -## 3. Generate the private key +## 3. Generate and transfer the private key On the app page: Private keys → Generate a private key. GitHub downloads one -PEM. Store it only on the controller host, root-owned `0600`. It is never -committed, printed, or copied elsewhere — see [SECRETS.md](SECRETS.md). +PEM to the management workstation. Treat that download as a temporary copy: +transfer it over an authenticated, encrypted channel directly to the final +root-owned path on the controller. Do not stage it in a shared directory or +send it through chat, email, or a repository. + +For example, from the management workstation (replace both placeholders): + +```bash +PEM="$HOME/Downloads/YOUR-APP.private-key.pem" +CONTROLLER=root@CONTROLLER_HOST +ssh "$CONTROLLER" \ + 'install -d -m 0700 /etc/ci-fleet/secrets && + umask 077 && cat > /etc/ci-fleet/secrets/github-app.pem && + chown root:root /etc/ci-fleet/secrets/github-app.pem && + chmod 0600 /etc/ci-fleet/secrets/github-app.pem' <"$PEM" +local_sha=$(sha256sum -- "$PEM" | cut -d' ' -f1) +remote_sha=$(ssh "$CONTROLLER" \ + "sha256sum /etc/ci-fleet/secrets/github-app.pem | cut -d' ' -f1") +test "$local_sha" = "$remote_sha" +ssh "$CONTROLLER" \ + "test \"\$(stat -c '%U:%G %a' /etc/ci-fleet/secrets/github-app.pem)\" = 'root:root 600'" +rm -f -- "$PEM" +unset PEM local_sha remote_sha +``` + +Use an equivalent privileged SSH workflow if direct root login is disabled. +Do not delete the workstation copy until both checksum and ownership/mode +checks pass. Then remove it from the browser download location, trash, sync, +and temporary storage according to the workstation's secure-erasure policy; +plain `rm` may not erase data from snapshots, SSDs, or copy-on-write storage. +The only retained copy may be the controller file or an approved secret +manager — see [SECRETS.md](SECRETS.md). ## 4. Install the app @@ -75,26 +105,37 @@ The controller exchanges a short-lived JWT signed with the PEM for an installation token at runtime (`scripts/github-app-token.sh`). No token is stored. -## 6. Verify +## 6. Pre-install verification from the reviewed checkout -On the controller host: +Before the first managed installation, `/opt/ci-fleet/manager/current` and the +installed-state file do not exist. On the controller host, run the token helper +from the exact reviewed engine checkout that will be installed: ```bash -sudo /opt/ci-fleet/manager/current/scripts/github-app-token.sh \ - --env-file /etc/ci-fleet/host.env +sudo /PATH/TO/REVIEWED/ci-fleet/scripts/github-app-token.sh \ + --env-file /etc/ci-fleet/host.env >/dev/null ``` -Prints nothing secret; exit 0 means JWT signing and token exchange work. -Then a check-only reconcile validates the full fetch path without applying -anything: +The helper writes the installation token to stdout, so the redirection is +mandatory. Exit 0 means JWT signing and token exchange work; it does not prove +that an installed controller can reconcile. Continue with the managed install +workflow before using an installed-manager command. + +## 7. Post-install remote-reconciliation verification + +Only after a successful managed install, validate the complete fetch and +installed-state path without applying anything: ```bash -sudo /opt/ci-fleet/manager/current/scripts/remote-reconcile.sh --check-only +sudo /opt/ci-fleet/manager/current/scripts/remote-reconcile.sh \ + --check-only --installed-ref ``` -`CHECK_OK` means the app can read the desired-state repository. A 403 or -"Repository not found" means the installation lacks the repository or the -`contents: read` grant — see Troubleshooting. +`CHECK_OK` means the app can read and validate the installed desired-state +commit and the controller is converged. The reconcile script consumes its +token internally; it does not print the token. A 403 or "Repository not found" +means the installation lacks the repository or the `contents: read` grant — +see Troubleshooting. ## Troubleshooting @@ -104,9 +145,76 @@ sudo /opt/ci-fleet/manager/current/scripts/remote-reconcile.sh --check-only | 403 on content API after granting permission | permission change saved on the app but not yet accepted on the installation — reopen the installation page and approve the pending permission request | | 401 on token exchange | wrong client ID, installation ID, or PEM path in `host.env` | -## Rotation and removal - -- New controller: new app. Do not share one app across controllers. -- Rotate: generate a new key, update the PEM path, delete the old key. -- Retire: uninstall the app from the organization. The host keeps no usable - credential. +## Key rotation: activate and verify before revocation + +New controller: new app. Do not share one app across controllers. + +1. Generate a new key and use the secure-transfer procedure in section 3 to + place it at a **new** root-owned `0600` path. Verify the transfer and remove + the downloaded workstation copy. +2. Update `CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE` in + `/etc/ci-fleet/host.env` to the new path. +3. Verify that the new key can mint a token, always suppressing token stdout: + + ```bash + sudo /opt/ci-fleet/manager/current/scripts/github-app-token.sh \ + --env-file /etc/ci-fleet/host.env >/dev/null + ``` + +4. Run a normal reconciliation, not `--check-only`. The host-configuration + drift forces the installer upgrade path, recreates the controller with the + new PEM mount, and runs its post-activation health check: + + ```bash + sudo /opt/ci-fleet/manager/current/scripts/remote-reconcile.sh + sudo /opt/ci-fleet/current/scripts/healthcheck.sh + sudo /opt/ci-fleet/manager/current/scripts/remote-reconcile.sh \ + --check-only --installed-ref + ``` + + Stop if reconciliation does not report `RECONCILE_OK`, the health check is + not healthy, or the final check does not report `CHECK_OK`. Restore the old + `host.env` path and reconcile again; do not revoke the old key. + +## Old-key revocation + +Only after every activation check above succeeds: + +1. On the GitHub App settings page, under **Private keys**, delete/revoke the + old key. +2. Remove the old PEM from the controller by its exact path; do not use a broad + wildcard in a shared secrets directory: + + ```bash + sudo rm -f -- /etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem + ``` + +3. Remove any old workstation or temporary copies under the applicable secure + erasure policy. Keep the new key and its configured path unchanged. + +## Controller retirement and PEM removal + +Retirement is not complete when only the App installation is removed: + +1. Drain the controller through reviewed desired state and verify zero managed + runners and zero effective capacity. +2. Uninstall the App from the organization to invalidate installation access. +3. On the GitHub App settings page, delete/revoke **every** private key for this + controller's app; delete the dedicated app itself if it will not be reused. +4. Uninstall the controller. The uninstaller deliberately preserves + `/etc/ci-fleet/host.env` and `/etc/ci-fleet/secrets`, so remove the retained + credentials explicitly: + + ```bash + sudo /opt/ci-fleet/manager/current/scripts/install-worker-controller.sh \ + --uninstall + sudo rm -f -- /etc/ci-fleet/secrets/github-app.pem + sudo rm -f -- /etc/ci-fleet/host.env + ``` + + Repeat the PEM removal for every exact rotation path used by this app. +5. Remove remaining management-workstation, temporary, secret-manager, and + backup copies according to their retention and secure-erasure policies. If + the retired storage cannot guarantee file-level erasure (for example SSD, + snapshot, or copy-on-write media), destroy the encrypted volume or its + encryption key before disposal. diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 55a82a24..abf35774 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -1,7 +1,9 @@ #!/usr/bin/env python3 +import re from pathlib import Path -raw_quickstart = (Path(__file__).resolve().parents[1] / "docs" / "QUICKSTART.md").read_text() +repo_root = Path(__file__).resolve().parents[1] +raw_quickstart = (repo_root / "docs" / "QUICKSTART.md").read_text() quickstart = " ".join(raw_quickstart.split()) required = ( @@ -16,4 +18,16 @@ assert quickstart.index("Cancel every queued job") < quickstart.index("3. Authorize the repository") assert "PROJECT_PREFIX=" not in raw_quickstart assert "managed controller managed controller" not in quickstart -print("quickstart_contract=PASS") + +app_setup = (repo_root / "docs" / "GITHUB-APP-SETUP.md").read_text() +token_calls = app_setup.count("scripts/github-app-token.sh \\") +redirected_token_calls = re.findall( + r"scripts/github-app-token\.sh \\\n\s+--env-file [^\n]+ >/dev/null", + app_setup, +) +assert token_calls == 2, f"expected two documented token-helper calls, found {token_calls}" +assert len(redirected_token_calls) == token_calls, "token-helper stdout must be redirected" +assert app_setup.index("## Key rotation: activate and verify before revocation") < app_setup.index( + "## Old-key revocation" +) +print("documentation_contract=PASS") From 4b63bc0003037fe0d44477a2b647fbeefa9c21bc Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:50:33 -0500 Subject: [PATCH 02/25] docs: make GitHub App key transfer rotation-safe --- docs/GITHUB-APP-SETUP.md | 94 +++++++++++++++++++++++++++----------- scripts/test_quickstart.py | 40 ++++++++++++++++ 2 files changed, 108 insertions(+), 26 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index f521da50..0f48c312 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -52,33 +52,71 @@ transfer it over an authenticated, encrypted channel directly to the final root-owned path on the controller. Do not stage it in a shared directory or send it through chat, email, or a repository. -For example, from the management workstation (replace both placeholders): +For an initial installation, the destination may use the conventional active +path because no controller key exists yet. Set `PEM_DEST` explicitly when a +different installation path is wanted: ```bash PEM="$HOME/Downloads/YOUR-APP.private-key.pem" CONTROLLER=root@CONTROLLER_HOST +ACTIVE_PEM="" +PEM_DEST=${PEM_DEST:-"/etc/ci-fleet/secrets/github-app.pem"} +``` + +For rotation, replace the last two assignments with the exact active path from +`CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE` and a new key-specific filename. Never +use the active path as the rotation destination: + +```bash +ACTIVE_PEM="/etc/ci-fleet/secrets/github-app.pem" +PEM_DEST="/etc/ci-fleet/secrets/github-app-ROTATION-ID.pem" +``` + +Then run the same transfer and verification sequence for either case. The path +validation makes it safe to quote `PEM_DEST` in the remote shell command, the +rotation guard rejects the active path, and shell noclobber prevents replacing +any existing file: + +```bash +[[ "$PEM_DEST" =~ ^/etc/ci-fleet/secrets/[A-Za-z0-9._-]+\.pem$ ]] || exit 1 +if [[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]; then + printf 'refusing to overwrite active PEM: %s\n' "$ACTIVE_PEM" >&2 + exit 1 +fi + +if ssh "$CONTROLLER" \ 'install -d -m 0700 /etc/ci-fleet/secrets && - umask 077 && cat > /etc/ci-fleet/secrets/github-app.pem && - chown root:root /etc/ci-fleet/secrets/github-app.pem && - chmod 0600 /etc/ci-fleet/secrets/github-app.pem' <"$PEM" -local_sha=$(sha256sum -- "$PEM" | cut -d' ' -f1) -remote_sha=$(ssh "$CONTROLLER" \ - "sha256sum /etc/ci-fleet/secrets/github-app.pem | cut -d' ' -f1") -test "$local_sha" = "$remote_sha" -ssh "$CONTROLLER" \ - "test \"\$(stat -c '%U:%G %a' /etc/ci-fleet/secrets/github-app.pem)\" = 'root:root 600'" -rm -f -- "$PEM" -unset PEM local_sha remote_sha + umask 077 && set -C && cat > "'"$PEM_DEST"'" && + chown root:root "'"$PEM_DEST"'" && + chmod 0600 "'"$PEM_DEST"'"' <"$PEM" && + local_sha=$(sha256sum -- "$PEM" | cut -d' ' -f1) && + remote_sha=$(ssh "$CONTROLLER" \ + "sha256sum -- \"$PEM_DEST\" | cut -d' ' -f1") && + test "$local_sha" = "$remote_sha" && + ssh "$CONTROLLER" \ + "test \"\$(stat -c '%U:%G' -- \"$PEM_DEST\")\" = 'root:root'" && + ssh "$CONTROLLER" \ + "test \"\$(stat -c '%a' -- \"$PEM_DEST\")\" = '600'" +then + rm -f -- "$PEM" + unset PEM local_sha remote_sha +else + printf 'transfer verification failed; retained downloaded PEM: %s\n' "$PEM" >&2 + unset local_sha remote_sha + exit 1 +fi ``` Use an equivalent privileged SSH workflow if direct root login is disabled. -Do not delete the workstation copy until both checksum and ownership/mode -checks pass. Then remove it from the browser download location, trash, sync, -and temporary storage according to the workstation's secure-erasure policy; -plain `rm` may not erase data from snapshots, SSDs, or copy-on-write storage. -The only retained copy may be the controller file or an approved secret -manager — see [SECRETS.md](SECRETS.md). +If transfer, checksum, owner, or mode verification fails, the sequence stops, +leaves the active controller PEM untouched, and retains the downloaded +replacement for diagnosis or a safe retry. Do not revoke the old GitHub key. +After success, remove any other copy from the browser download location, trash, +sync, and temporary storage according to the workstation's secure-erasure +policy; plain `rm` may not erase data from snapshots, SSDs, or copy-on-write +storage. The only retained copy may be the controller file or an approved +secret manager — see [SECRETS.md](SECRETS.md). ## 4. Install the app @@ -149,11 +187,13 @@ see Troubleshooting. New controller: new app. Do not share one app across controllers. -1. Generate a new key and use the secure-transfer procedure in section 3 to - place it at a **new** root-owned `0600` path. Verify the transfer and remove - the downloaded workstation copy. +1. Generate a new key and use the rotation assignments and secure-transfer + procedure in section 3. `PEM_DEST` must differ from `ACTIVE_PEM`; the + transfer must not overwrite either path. Continue only after every transfer + and verification command succeeds and the downloaded workstation copy is + removed. 2. Update `CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE` in - `/etc/ci-fleet/host.env` to the new path. + `/etc/ci-fleet/host.env` to the exact value of `PEM_DEST`. 3. Verify that the new key can mint a token, always suppressing token stdout: ```bash @@ -173,8 +213,10 @@ New controller: new app. Do not share one app across controllers. ``` Stop if reconciliation does not report `RECONCILE_OK`, the health check is - not healthy, or the final check does not report `CHECK_OK`. Restore the old - `host.env` path and reconcile again; do not revoke the old key. + not healthy, or the final check does not report `CHECK_OK`. Restore + `CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE` to the exact value of `ACTIVE_PEM` + and reconcile again; do not revoke the old key or remove either PEM until + rollback is healthy and converged. ## Old-key revocation @@ -186,7 +228,7 @@ Only after every activation check above succeeds: wildcard in a shared secrets directory: ```bash - sudo rm -f -- /etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem + sudo rm -f -- "$ACTIVE_PEM" ``` 3. Remove any old workstation or temporary copies under the applicable secure @@ -208,7 +250,7 @@ Retirement is not complete when only the App installation is removed: ```bash sudo /opt/ci-fleet/manager/current/scripts/install-worker-controller.sh \ --uninstall - sudo rm -f -- /etc/ci-fleet/secrets/github-app.pem + sudo rm -f -- "$PEM_DEST" sudo rm -f -- /etc/ci-fleet/host.env ``` diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index abf35774..4d158f87 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -30,4 +30,44 @@ assert app_setup.index("## Key rotation: activate and verify before revocation") < app_setup.index( "## Old-key revocation" ) + +rotation = re.search( + r'ACTIVE_PEM="(/etc/ci-fleet/secrets/[^"\n]+)"\n' + r'PEM_DEST="(/etc/ci-fleet/secrets/[^"\n]+)"', + app_setup, +) +assert rotation and rotation[1] != rotation[2], "rotation destination must differ from active PEM" + +transfer = app_setup[ + app_setup.index('[[ "$PEM_DEST" =~') : app_setup.index( + "Use an equivalent privileged SSH workflow" + ) +] +active_guard = '[[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]' +noclobber_line = next(line for line in transfer.splitlines() if "set -C && cat >" in line) +assert '"$PEM_DEST"' in noclobber_line +assert active_guard in transfer +assert transfer.index(active_guard) < transfer.index('ssh "$CONTROLLER"') +for command in ("cat >", "sha256sum --", "stat -c '%U:%G'", "stat -c '%a'"): + assert any(command in line and "$PEM_DEST" in line for line in transfer.splitlines()), ( + f"transfer does not use configured destination: {command}" + ) + +success_branch, failure_branch = transfer.split("else", 1) +for check in ( + 'test "$local_sha" = "$remote_sha" &&\n', + "= 'root:root'\" &&\n", + "= '600'\"\nthen", +): + assert check in success_branch, f"download deletion is not gated by: {check}" + assert success_branch.index(check) < success_branch.index('rm -f -- "$PEM"') +assert 'rm -f -- "$PEM"' not in failure_branch + +for use in ( + "exact value of `PEM_DEST`", + "exact value of `ACTIVE_PEM`", + 'sudo rm -f -- "$ACTIVE_PEM"', + 'sudo rm -f -- "$PEM_DEST"', +): + assert use in app_setup, f"configured PEM destination contract missing: {use}" print("documentation_contract=PASS") From 62d16099a44e8398deadd368b107912aed2d3951 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:02:28 -0500 Subject: [PATCH 03/25] docs: preserve PEM paths through retirement --- docs/GITHUB-APP-SETUP.md | 24 +++++++++++++++++++----- scripts/test_quickstart.py | 13 ++++++++++++- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 0f48c312..648e47a0 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -243,18 +243,32 @@ Retirement is not complete when only the App installation is removed: 2. Uninstall the App from the organization to invalidate installation access. 3. On the GitHub App settings page, delete/revoke **every** private key for this controller's app; delete the dedicated app itself if it will not be reused. -4. Uninstall the controller. The uninstaller deliberately preserves - `/etc/ci-fleet/host.env` and `/etc/ci-fleet/secrets`, so remove the retained - credentials explicitly: +4. Before uninstalling, read the configured destination while `host.env` still + exists and enumerate every retained rotation path explicitly. The + uninstaller deliberately preserves `/etc/ci-fleet/host.env` and + `/etc/ci-fleet/secrets`: ```bash + PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ + /etc/ci-fleet/host.env | cut -d= -f2-) + RETIRED_PEMS=("$PEM_DEST") + # Repeat for every retained old rotation path; never use a wildcard. + RETIRED_PEMS+=("/etc/ci-fleet/secrets/OLD-ROTATION-ID.pem") + for pem in "${RETIRED_PEMS[@]}"; do + [[ "$pem" =~ ^/etc/ci-fleet/secrets/[A-Za-z0-9._-]+\.pem$ ]] || exit 1 + done + sudo /opt/ci-fleet/manager/current/scripts/install-worker-controller.sh \ --uninstall - sudo rm -f -- "$PEM_DEST" + for pem in "${RETIRED_PEMS[@]}"; do + sudo rm -f -- "$pem" + done sudo rm -f -- /etc/ci-fleet/host.env ``` - Repeat the PEM removal for every exact rotation path used by this app. + Replace or repeat the example old path for every exact rotation path used by + this app. If path extraction or validation fails, stop before uninstalling + or removing `host.env`. 5. Remove remaining management-workstation, temporary, secret-manager, and backup copies according to their retention and secure-erasure policies. If the retired storage cannot guarantee file-level erasure (for example SSD, diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 4d158f87..fa86552c 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -67,7 +67,18 @@ "exact value of `PEM_DEST`", "exact value of `ACTIVE_PEM`", 'sudo rm -f -- "$ACTIVE_PEM"', - 'sudo rm -f -- "$PEM_DEST"', ): assert use in app_setup, f"configured PEM destination contract missing: {use}" + +retirement = app_setup[app_setup.index("## Controller retirement and PEM removal") :] +read_destination = "PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE='" +validate_paths = '[[ "$pem" =~ ^/etc/ci-fleet/secrets/' +uninstall = "scripts/install-worker-controller.sh \\\n --uninstall" +remove_pem = 'sudo rm -f -- "$pem"' +remove_host_env = "sudo rm -f -- /etc/ci-fleet/host.env" +assert 'RETIRED_PEMS=("$PEM_DEST")' in retirement +assert retirement.index(read_destination) < retirement.index(validate_paths) +assert retirement.index(validate_paths) < retirement.index(uninstall) +assert retirement.index(uninstall) < retirement.index(remove_pem) +assert retirement.index(remove_pem) < retirement.index(remove_host_env) print("documentation_contract=PASS") From e32a1046dccecb57bfdf495191338494207e37a9 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:10:51 -0500 Subject: [PATCH 04/25] docs: validate old PEM cleanup path --- docs/GITHUB-APP-SETUP.md | 19 +++++++++++++++---- scripts/test_quickstart.py | 16 ++++++++++++++-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 648e47a0..777f7a65 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -192,8 +192,10 @@ New controller: new app. Do not share one app across controllers. transfer must not overwrite either path. Continue only after every transfer and verification command succeeds and the downloaded workstation copy is removed. -2. Update `CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE` in - `/etc/ci-fleet/host.env` to the exact value of `PEM_DEST`. +2. Before changing `host.env`, record the exact value of `ACTIVE_PEM` for + rollback and old-file removal. Then update + `CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE` in `/etc/ci-fleet/host.env` to the + exact value of `PEM_DEST`. 3. Verify that the new key can mint a token, always suppressing token stdout: ```bash @@ -224,10 +226,19 @@ Only after every activation check above succeeds: 1. On the GitHub App settings page, under **Private keys**, delete/revoke the old key. -2. Remove the old PEM from the controller by its exact path; do not use a broad - wildcard in a shared secrets directory: +2. In the current shell, read the new active destination from `host.env` and + reassign `ACTIVE_PEM` to the exact old path recorded before activation. + Validate both paths and their inequality before removing the old PEM; do not + use a broad wildcard in a shared secrets directory: ```bash + PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ + /etc/ci-fleet/host.env | cut -d= -f2-) + ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem" + for pem in "$PEM_DEST" "$ACTIVE_PEM"; do + [[ "$pem" =~ ^/etc/ci-fleet/secrets/[A-Za-z0-9._-]+\.pem$ ]] || exit 1 + done + [[ "$ACTIVE_PEM" != "$PEM_DEST" ]] || exit 1 sudo rm -f -- "$ACTIVE_PEM" ``` diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index fa86552c..20680f6d 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -66,12 +66,24 @@ for use in ( "exact value of `PEM_DEST`", "exact value of `ACTIVE_PEM`", - 'sudo rm -f -- "$ACTIVE_PEM"', ): assert use in app_setup, f"configured PEM destination contract missing: {use}" -retirement = app_setup[app_setup.index("## Controller retirement and PEM removal") :] +revocation = app_setup[ + app_setup.index("## Old-key revocation") : app_setup.index( + "## Controller retirement and PEM removal" + ) +] read_destination = "PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE='" +validate_paths = 'for pem in "$PEM_DEST" "$ACTIVE_PEM"; do' +distinct_paths = '[[ "$ACTIVE_PEM" != "$PEM_DEST" ]] || exit 1' +remove_old_pem = 'sudo rm -f -- "$ACTIVE_PEM"' +assert 'ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem"' in revocation +assert revocation.index(read_destination) < revocation.index(validate_paths) +assert revocation.index(validate_paths) < revocation.index(distinct_paths) +assert revocation.index(distinct_paths) < revocation.index(remove_old_pem) + +retirement = app_setup[app_setup.index("## Controller retirement and PEM removal") :] validate_paths = '[[ "$pem" =~ ^/etc/ci-fleet/secrets/' uninstall = "scripts/install-worker-controller.sh \\\n --uninstall" remove_pem = 'sudo rm -f -- "$pem"' From 658eda7c94fade9cd1fcea0a5326648ecf577602 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:24:18 -0500 Subject: [PATCH 05/25] docs: fail closed across PEM cleanup --- docs/GITHUB-APP-SETUP.md | 52 +++++++++++++++++++++++++++++--------- scripts/test_quickstart.py | 41 ++++++++++++++++++++---------- 2 files changed, 68 insertions(+), 25 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 777f7a65..f95a3781 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -78,15 +78,22 @@ rotation guard rejects the active path, and shell noclobber prevents replacing any existing file: ```bash -[[ "$PEM_DEST" =~ ^/etc/ci-fleet/secrets/[A-Za-z0-9._-]+\.pem$ ]] || exit 1 +valid_pem_path() { + [[ $1 =~ ^/[A-Za-z0-9._/-]+\.pem$ && $1 != *//* && + ! $1 =~ (^|/)\.\.?(/|$) ]] +} +valid_pem_path "$PEM_DEST" || exit 1 if [[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]; then printf 'refusing to overwrite active PEM: %s\n' "$ACTIVE_PEM" >&2 exit 1 fi +PEM_DIR=${PEM_DEST%/*} +# PEM_DEST is validated above and intentionally expanded client-side. +# shellcheck disable=SC2029 if ssh "$CONTROLLER" \ - 'install -d -m 0700 /etc/ci-fleet/secrets && + 'install -d -m 0700 "'"$PEM_DIR"'" && umask 077 && set -C && cat > "'"$PEM_DEST"'" && chown root:root "'"$PEM_DEST"'" && chmod 0600 "'"$PEM_DEST"'"' <"$PEM" && @@ -99,8 +106,13 @@ ssh "$CONTROLLER" \ ssh "$CONTROLLER" \ "test \"\$(stat -c '%a' -- \"$PEM_DEST\")\" = '600'" then - rm -f -- "$PEM" - unset PEM local_sha remote_sha + if rm -f -- "$PEM"; then + unset PEM local_sha remote_sha + else + printf 'verified transfer, but could not delete downloaded PEM: %s\n' \ + "$PEM" >&2 + exit 1 + fi else printf 'transfer verification failed; retained downloaded PEM: %s\n' "$PEM" >&2 unset local_sha remote_sha @@ -232,11 +244,15 @@ Only after every activation check above succeeds: use a broad wildcard in a shared secrets directory: ```bash + valid_pem_path() { + [[ $1 =~ ^/[A-Za-z0-9._/-]+\.pem$ && $1 != *//* && + ! $1 =~ (^|/)\.\.?(/|$) ]] + } PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem" for pem in "$PEM_DEST" "$ACTIVE_PEM"; do - [[ "$pem" =~ ^/etc/ci-fleet/secrets/[A-Za-z0-9._-]+\.pem$ ]] || exit 1 + valid_pem_path "$pem" || exit 1 done [[ "$ACTIVE_PEM" != "$PEM_DEST" ]] || exit 1 sudo rm -f -- "$ACTIVE_PEM" @@ -260,21 +276,33 @@ Retirement is not complete when only the App installation is removed: `/etc/ci-fleet/secrets`: ```bash + valid_pem_path() { + [[ $1 =~ ^/[A-Za-z0-9._/-]+\.pem$ && $1 != *//* && + ! $1 =~ (^|/)\.\.?(/|$) ]] + } PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) RETIRED_PEMS=("$PEM_DEST") # Repeat for every retained old rotation path; never use a wildcard. RETIRED_PEMS+=("/etc/ci-fleet/secrets/OLD-ROTATION-ID.pem") for pem in "${RETIRED_PEMS[@]}"; do - [[ "$pem" =~ ^/etc/ci-fleet/secrets/[A-Za-z0-9._-]+\.pem$ ]] || exit 1 + valid_pem_path "$pem" || exit 1 done - sudo /opt/ci-fleet/manager/current/scripts/install-worker-controller.sh \ - --uninstall - for pem in "${RETIRED_PEMS[@]}"; do - sudo rm -f -- "$pem" - done - sudo rm -f -- /etc/ci-fleet/host.env + remove_retired_pems() { + for pem in "${RETIRED_PEMS[@]}"; do + sudo rm -f -- "$pem" || return 1 + done + } + if sudo /opt/ci-fleet/manager/current/scripts/install-worker-controller.sh \ + --uninstall && + remove_retired_pems && + sudo rm -f -- /etc/ci-fleet/host.env; then + unset PEM_DEST RETIRED_PEMS + else + printf 'retirement cleanup failed; retained path inventory and host.env\n' >&2 + exit 1 + fi ``` Replace or repeat the example old path for every exact rotation path used by diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 20680f6d..10ad895f 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -39,13 +39,16 @@ assert rotation and rotation[1] != rotation[2], "rotation destination must differ from active PEM" transfer = app_setup[ - app_setup.index('[[ "$PEM_DEST" =~') : app_setup.index( + app_setup.index("valid_pem_path() {") : app_setup.index( "Use an equivalent privileged SSH workflow" ) ] active_guard = '[[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]' noclobber_line = next(line for line in transfer.splitlines() if "set -C && cat >" in line) assert '"$PEM_DEST"' in noclobber_line +assert "valid_pem_path \"$PEM_DEST\" || exit 1" in transfer +assert 'install -d -m 0700 "\'"$PEM_DIR"\'"' in transfer +assert "^/[A-Za-z0-9._/-]+\\.pem$" in transfer assert active_guard in transfer assert transfer.index(active_guard) < transfer.index('ssh "$CONTROLLER"') for command in ("cat >", "sha256sum --", "stat -c '%U:%G'", "stat -c '%a'"): @@ -53,15 +56,23 @@ f"transfer does not use configured destination: {command}" ) -success_branch, failure_branch = transfer.split("else", 1) +verification = transfer[transfer.index('if\nssh "$CONTROLLER"') : transfer.index("then\n if rm")] for check in ( 'test "$local_sha" = "$remote_sha" &&\n', "= 'root:root'\" &&\n", - "= '600'\"\nthen", + "= '600'\"\n", ): - assert check in success_branch, f"download deletion is not gated by: {check}" - assert success_branch.index(check) < success_branch.index('rm -f -- "$PEM"') -assert 'rm -f -- "$PEM"' not in failure_branch + assert check in verification, f"download deletion is not gated by: {check}" + +delete_download = transfer[ + transfer.index('if rm -f -- "$PEM"; then') : transfer.index( + "else\n printf 'transfer verification failed" + ) +] +delete_success, delete_failure = delete_download.split("else", 1) +assert "unset PEM" in delete_success +assert '"$PEM" >&2' in delete_failure and "exit 1" in delete_failure +assert 'rm -f -- "$PEM"' not in transfer[transfer.index("transfer verification failed") :] for use in ( "exact value of `PEM_DEST`", @@ -79,18 +90,22 @@ distinct_paths = '[[ "$ACTIVE_PEM" != "$PEM_DEST" ]] || exit 1' remove_old_pem = 'sudo rm -f -- "$ACTIVE_PEM"' assert 'ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem"' in revocation +assert "^/[A-Za-z0-9._/-]+\\.pem$" in revocation assert revocation.index(read_destination) < revocation.index(validate_paths) assert revocation.index(validate_paths) < revocation.index(distinct_paths) assert revocation.index(distinct_paths) < revocation.index(remove_old_pem) retirement = app_setup[app_setup.index("## Controller retirement and PEM removal") :] -validate_paths = '[[ "$pem" =~ ^/etc/ci-fleet/secrets/' -uninstall = "scripts/install-worker-controller.sh \\\n --uninstall" -remove_pem = 'sudo rm -f -- "$pem"' -remove_host_env = "sudo rm -f -- /etc/ci-fleet/host.env" +validate_paths = 'valid_pem_path "$pem" || exit 1' +uninstall = "scripts/install-worker-controller.sh \\\n --uninstall &&" +remove_pem = 'sudo rm -f -- "$pem" || return 1' +remove_host_env = "sudo rm -f -- /etc/ci-fleet/host.env; then" assert 'RETIRED_PEMS=("$PEM_DEST")' in retirement +assert "^/[A-Za-z0-9._/-]+\\.pem$" in retirement assert retirement.index(read_destination) < retirement.index(validate_paths) -assert retirement.index(validate_paths) < retirement.index(uninstall) -assert retirement.index(uninstall) < retirement.index(remove_pem) -assert retirement.index(remove_pem) < retirement.index(remove_host_env) +assert remove_pem in retirement +assert retirement.index(uninstall) < retirement.index("remove_retired_pems &&") +assert retirement.index("remove_retired_pems &&") < retirement.index(remove_host_env) +retirement_failure = retirement[retirement.index("retirement cleanup failed") :] +assert "path inventory and host.env" in retirement_failure and "exit 1" in retirement_failure print("documentation_contract=PASS") From 9801048a2b83cdffcd48f033ab54b774eacc3777 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:34:08 -0500 Subject: [PATCH 06/25] docs: preserve custom PEM lifecycle state --- docs/GITHUB-APP-SETUP.md | 23 +++++++++++++++-------- scripts/test_quickstart.py | 17 +++++++++++------ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index f95a3781..918e167d 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -79,7 +79,7 @@ any existing file: ```bash valid_pem_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+\.pem$ && $1 != *//* && + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ && $1 != *//* && ! $1 =~ (^|/)\.\.?(/|$) ]] } valid_pem_path "$PEM_DEST" || exit 1 @@ -93,7 +93,7 @@ PEM_DIR=${PEM_DEST%/*} # shellcheck disable=SC2029 if ssh "$CONTROLLER" \ - 'install -d -m 0700 "'"$PEM_DIR"'" && + '{ test -d "'"$PEM_DIR"'" || install -d -m 0700 "'"$PEM_DIR"'"; } && umask 077 && set -C && cat > "'"$PEM_DEST"'" && chown root:root "'"$PEM_DEST"'" && chmod 0600 "'"$PEM_DEST"'"' <"$PEM" && @@ -245,7 +245,7 @@ Only after every activation check above succeeds: ```bash valid_pem_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+\.pem$ && $1 != *//* && + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ && $1 != *//* && ! $1 =~ (^|/)\.\.?(/|$) ]] } PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ @@ -277,7 +277,7 @@ Retirement is not complete when only the App installation is removed: ```bash valid_pem_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+\.pem$ && $1 != *//* && + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ && $1 != *//* && ! $1 =~ (^|/)\.\.?(/|$) ]] } PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ @@ -288,6 +288,9 @@ Retirement is not complete when only the App installation is removed: for pem in "${RETIRED_PEMS[@]}"; do valid_pem_path "$pem" || exit 1 done + PEM_INVENTORY=/etc/ci-fleet/retired-pem-paths + printf '%s\n' "${RETIRED_PEMS[@]}" | \ + sudo install -m 0600 /dev/stdin "$PEM_INVENTORY" || exit 1 remove_retired_pems() { for pem in "${RETIRED_PEMS[@]}"; do @@ -297,17 +300,21 @@ Retirement is not complete when only the App installation is removed: if sudo /opt/ci-fleet/manager/current/scripts/install-worker-controller.sh \ --uninstall && remove_retired_pems && - sudo rm -f -- /etc/ci-fleet/host.env; then - unset PEM_DEST RETIRED_PEMS + sudo rm -f -- /etc/ci-fleet/host.env && + sudo rm -f -- "$PEM_INVENTORY"; then + unset PEM_DEST PEM_INVENTORY RETIRED_PEMS else - printf 'retirement cleanup failed; retained path inventory and host.env\n' >&2 + printf 'retirement cleanup failed; retained %s and host.env\n' \ + "$PEM_INVENTORY" >&2 exit 1 fi ``` Replace or repeat the example old path for every exact rotation path used by this app. If path extraction or validation fails, stop before uninstalling - or removing `host.env`. + or removing `host.env`. On later cleanup failure, rebuild `RETIRED_PEMS` + from the retained root-owned inventory before retrying exact-path removal; + the inventory is deleted only after every PEM and `host.env` are removed. 5. Remove remaining management-workstation, temporary, secret-manager, and backup copies according to their retention and secure-erasure policies. If the retired storage cannot guarantee file-level erasure (for example SSD, diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 10ad895f..8e9fe62f 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -47,8 +47,8 @@ noclobber_line = next(line for line in transfer.splitlines() if "set -C && cat >" in line) assert '"$PEM_DEST"' in noclobber_line assert "valid_pem_path \"$PEM_DEST\" || exit 1" in transfer -assert 'install -d -m 0700 "\'"$PEM_DIR"\'"' in transfer -assert "^/[A-Za-z0-9._/-]+\\.pem$" in transfer +assert 'test -d "\'"$PEM_DIR"\'" || install -d -m 0700 "\'"$PEM_DIR"\'"' in transfer +assert "^/[A-Za-z0-9._/-]+$" in transfer assert active_guard in transfer assert transfer.index(active_guard) < transfer.index('ssh "$CONTROLLER"') for command in ("cat >", "sha256sum --", "stat -c '%U:%G'", "stat -c '%a'"): @@ -90,7 +90,7 @@ distinct_paths = '[[ "$ACTIVE_PEM" != "$PEM_DEST" ]] || exit 1' remove_old_pem = 'sudo rm -f -- "$ACTIVE_PEM"' assert 'ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem"' in revocation -assert "^/[A-Za-z0-9._/-]+\\.pem$" in revocation +assert "^/[A-Za-z0-9._/-]+$" in revocation assert revocation.index(read_destination) < revocation.index(validate_paths) assert revocation.index(validate_paths) < revocation.index(distinct_paths) assert revocation.index(distinct_paths) < revocation.index(remove_old_pem) @@ -99,13 +99,18 @@ validate_paths = 'valid_pem_path "$pem" || exit 1' uninstall = "scripts/install-worker-controller.sh \\\n --uninstall &&" remove_pem = 'sudo rm -f -- "$pem" || return 1' -remove_host_env = "sudo rm -f -- /etc/ci-fleet/host.env; then" +persist_inventory = 'sudo install -m 0600 /dev/stdin "$PEM_INVENTORY" || exit 1' +remove_host_env = "sudo rm -f -- /etc/ci-fleet/host.env &&" +remove_inventory = 'sudo rm -f -- "$PEM_INVENTORY"; then' assert 'RETIRED_PEMS=("$PEM_DEST")' in retirement -assert "^/[A-Za-z0-9._/-]+\\.pem$" in retirement +assert "^/[A-Za-z0-9._/-]+$" in retirement assert retirement.index(read_destination) < retirement.index(validate_paths) +assert retirement.index(validate_paths) < retirement.index(persist_inventory) +assert retirement.index(persist_inventory) < retirement.index(uninstall) assert remove_pem in retirement assert retirement.index(uninstall) < retirement.index("remove_retired_pems &&") assert retirement.index("remove_retired_pems &&") < retirement.index(remove_host_env) +assert retirement.index(remove_host_env) < retirement.index(remove_inventory) retirement_failure = retirement[retirement.index("retirement cleanup failed") :] -assert "path inventory and host.env" in retirement_failure and "exit 1" in retirement_failure +assert '"$PEM_INVENTORY" >&2' in retirement_failure and "exit 1" in retirement_failure print("documentation_contract=PASS") From 23b00794b80fb8ea29d01ba2dc12f5a3ce3e61ef Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:40:18 -0500 Subject: [PATCH 07/25] docs: handle root-level PEM destinations --- docs/GITHUB-APP-SETUP.md | 1 + scripts/test_quickstart.py | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 918e167d..baa7e8f9 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -88,6 +88,7 @@ if [[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]; then exit 1 fi PEM_DIR=${PEM_DEST%/*} +[[ -n "$PEM_DIR" ]] || PEM_DIR=/ # PEM_DEST is validated above and intentionally expanded client-side. # shellcheck disable=SC2029 diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 8e9fe62f..cbf147ff 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -47,6 +47,7 @@ noclobber_line = next(line for line in transfer.splitlines() if "set -C && cat >" in line) assert '"$PEM_DEST"' in noclobber_line assert "valid_pem_path \"$PEM_DEST\" || exit 1" in transfer +assert '[[ -n "$PEM_DIR" ]] || PEM_DIR=/' in transfer assert 'test -d "\'"$PEM_DIR"\'" || install -d -m 0700 "\'"$PEM_DIR"\'"' in transfer assert "^/[A-Za-z0-9._/-]+$" in transfer assert active_guard in transfer From 7eed9e777b3ed65c8b6bfc70b3af9c7919326df0 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:54:26 -0500 Subject: [PATCH 08/25] docs: close PEM lifecycle cleanup gaps --- docs/GITHUB-APP-SETUP.md | 72 ++++++++++++++++++++++++++++---------- scripts/test_quickstart.py | 24 +++++++++++-- 2 files changed, 75 insertions(+), 21 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index baa7e8f9..3b2e6371 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -228,10 +228,30 @@ New controller: new app. Do not share one app across controllers. ``` Stop if reconciliation does not report `RECONCILE_OK`, the health check is - not healthy, or the final check does not report `CHECK_OK`. Restore + not `healthy` for an `active` controller or `maintenance` for a controller + whose reviewed desired state is `drained` or `disabled`, or the final check + does not report `CHECK_OK`. Warning and unhealthy results always fail. + Restore `CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE` to the exact value of `ACTIVE_PEM` and reconcile again; do not revoke the old key or remove either PEM until rollback is healthy and converged. +5. Before revoking the old key, verify that the newest complete rollback + checkpoint references `PEM_DEST`, not `ACTIVE_PEM`: + + ```bash + LATEST_CHECKPOINT=$(sudo find /var/lib/ci-fleet/checkpoints \ + -mindepth 2 -maxdepth 2 -type f -name .complete \ + -printf '%T@ %h\n' | sort -nr | awk 'NR == 1 {print $2}') + [[ -n "$LATEST_CHECKPOINT" ]] || exit 1 + sudo grep -Fx -- \ + "CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=$PEM_DEST" \ + "$LATEST_CHECKPOINT/ci-fleet.env" >/dev/null || exit 1 + ``` + + The rotation itself checkpoints the pre-rotation environment. If this gate + fails, retain the old GitHub key and old PEM until a subsequent reviewed + controller mutation creates and validates a checkpoint based on the new + path. Do not revoke a key still required by the latest rollback checkpoint. ## Old-key revocation @@ -272,9 +292,11 @@ Retirement is not complete when only the App installation is removed: 3. On the GitHub App settings page, delete/revoke **every** private key for this controller's app; delete the dedicated app itself if it will not be reused. 4. Before uninstalling, read the configured destination while `host.env` still - exists and enumerate every retained rotation path explicitly. The - uninstaller deliberately preserves `/etc/ci-fleet/host.env` and - `/etc/ci-fleet/secrets`: + exists and classify every retained rotation path explicitly. Revoke or + unmount each secret-manager-backed path through that manager first and + verify that it is absent; never pass a manager-owned path to `rm`. + `LOCAL_PEMS` must contain only host-local files. The uninstaller deliberately + preserves `/etc/ci-fleet/host.env` and `/etc/ci-fleet/secrets`: ```bash valid_pem_path() { @@ -283,18 +305,31 @@ Retirement is not complete when only the App installation is removed: } PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) - RETIRED_PEMS=("$PEM_DEST") - # Repeat for every retained old rotation path; never use a wildcard. - RETIRED_PEMS+=("/etc/ci-fleet/secrets/OLD-ROTATION-ID.pem") - for pem in "${RETIRED_PEMS[@]}"; do + # Put PEM_DEST and every old path in exactly one array; never use a wildcard. + LOCAL_PEMS=("/etc/ci-fleet/secrets/HOST-LOCAL-KEY.pem") + MANAGED_PEMS=("/run/secret-manager/MANAGER-BACKED-KEY") + PEM_INVENTORY=/etc/ci-fleet/retired-pem-paths + configured_classifications=0 + for pem in "${LOCAL_PEMS[@]}" "${MANAGED_PEMS[@]}"; do valid_pem_path "$pem" || exit 1 + [[ "$pem" != "$PEM_INVENTORY" ]] || exit 1 + if [[ "$pem" == "$PEM_DEST" ]]; then + configured_classifications=$((configured_classifications + 1)) + fi done - PEM_INVENTORY=/etc/ci-fleet/retired-pem-paths - printf '%s\n' "${RETIRED_PEMS[@]}" | \ - sudo install -m 0600 /dev/stdin "$PEM_INVENTORY" || exit 1 + ((configured_classifications == 1)) || exit 1 + for pem in "${MANAGED_PEMS[@]}"; do + sudo test ! -e "$pem" || exit 1 + done + if ((${#LOCAL_PEMS[@]})); then + printf '%s\n' "${LOCAL_PEMS[@]}" | \ + sudo install -m 0600 /dev/stdin "$PEM_INVENTORY" + else + sudo install -m 0600 /dev/null "$PEM_INVENTORY" + fi || exit 1 remove_retired_pems() { - for pem in "${RETIRED_PEMS[@]}"; do + for pem in "${LOCAL_PEMS[@]}"; do sudo rm -f -- "$pem" || return 1 done } @@ -303,7 +338,7 @@ Retirement is not complete when only the App installation is removed: remove_retired_pems && sudo rm -f -- /etc/ci-fleet/host.env && sudo rm -f -- "$PEM_INVENTORY"; then - unset PEM_DEST PEM_INVENTORY RETIRED_PEMS + unset PEM_DEST PEM_INVENTORY LOCAL_PEMS MANAGED_PEMS else printf 'retirement cleanup failed; retained %s and host.env\n' \ "$PEM_INVENTORY" >&2 @@ -311,13 +346,14 @@ Retirement is not complete when only the App installation is removed: fi ``` - Replace or repeat the example old path for every exact rotation path used by - this app. If path extraction or validation fails, stop before uninstalling - or removing `host.env`. On later cleanup failure, rebuild `RETIRED_PEMS` + Replace or repeat the examples for every exact rotation path used by this + app. If classification, manager cleanup, extraction, or validation fails, + stop before uninstalling + or removing `host.env`. On later cleanup failure, rebuild `LOCAL_PEMS` from the retained root-owned inventory before retrying exact-path removal; the inventory is deleted only after every PEM and `host.env` are removed. -5. Remove remaining management-workstation, temporary, secret-manager, and - backup copies according to their retention and secure-erasure policies. If +5. Remove remaining management-workstation, temporary, and backup copies + according to their retention and secure-erasure policies. If the retired storage cannot guarantee file-level erasure (for example SSD, snapshot, or copy-on-write media), destroy the encrypted volume or its encryption key before disposal. diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index cbf147ff..0e79230f 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -81,6 +81,16 @@ ): assert use in app_setup, f"configured PEM destination contract missing: {use}" +rotation = app_setup[ + app_setup.index("## Key rotation: activate and verify before revocation") : + app_setup.index("## Old-key revocation") +] +assert "`healthy` for an `active` controller" in rotation +assert "`maintenance`" in rotation and "`drained` or `disabled`" in rotation +checkpoint_match = '"CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=$PEM_DEST"' +assert checkpoint_match in rotation +assert "retain the old GitHub key and old PEM" in rotation + revocation = app_setup[ app_setup.index("## Old-key revocation") : app_setup.index( "## Controller retirement and PEM removal" @@ -100,15 +110,23 @@ validate_paths = 'valid_pem_path "$pem" || exit 1' uninstall = "scripts/install-worker-controller.sh \\\n --uninstall &&" remove_pem = 'sudo rm -f -- "$pem" || return 1' -persist_inventory = 'sudo install -m 0600 /dev/stdin "$PEM_INVENTORY" || exit 1' +persist_inventory = 'sudo install -m 0600 /dev/stdin "$PEM_INVENTORY"' remove_host_env = "sudo rm -f -- /etc/ci-fleet/host.env &&" remove_inventory = 'sudo rm -f -- "$PEM_INVENTORY"; then' -assert 'RETIRED_PEMS=("$PEM_DEST")' in retirement +classify_destination = 'configured_classifications=$((configured_classifications + 1))' +manager_absent = 'sudo test ! -e "$pem" || exit 1' +inventory_distinct = '[[ "$pem" != "$PEM_INVENTORY" ]] || exit 1' +assert 'LOCAL_PEMS=(' in retirement and 'MANAGED_PEMS=(' in retirement assert "^/[A-Za-z0-9._/-]+$" in retirement assert retirement.index(read_destination) < retirement.index(validate_paths) -assert retirement.index(validate_paths) < retirement.index(persist_inventory) +assert retirement.index(validate_paths) < retirement.index(inventory_distinct) +assert retirement.index(inventory_distinct) < retirement.index(classify_destination) +assert "((configured_classifications == 1)) || exit 1" in retirement +assert retirement.index(classify_destination) < retirement.index(manager_absent) +assert retirement.index(manager_absent) < retirement.index(persist_inventory) assert retirement.index(persist_inventory) < retirement.index(uninstall) assert remove_pem in retirement +assert 'for pem in "${LOCAL_PEMS[@]}"; do' in retirement assert retirement.index(uninstall) < retirement.index("remove_retired_pems &&") assert retirement.index("remove_retired_pems &&") < retirement.index(remove_host_env) assert retirement.index(remove_host_env) < retirement.index(remove_inventory) From 7f54a7d91bf7a2df5c358e0ae3d8f174105c1c93 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:05:02 -0500 Subject: [PATCH 09/25] docs: match rollback checkpoint selection --- docs/GITHUB-APP-SETUP.md | 1 + scripts/test_quickstart.py | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 3b2e6371..20558863 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -241,6 +241,7 @@ New controller: new app. Do not share one app across controllers. ```bash LATEST_CHECKPOINT=$(sudo find /var/lib/ci-fleet/checkpoints \ -mindepth 2 -maxdepth 2 -type f -name .complete \ + ! -path '/var/lib/ci-fleet/checkpoints/.checkpoint.staging.*/*' \ -printf '%T@ %h\n' | sort -nr | awk 'NR == 1 {print $2}') [[ -n "$LATEST_CHECKPOINT" ]] || exit 1 sudo grep -Fx -- \ diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 0e79230f..ab51211d 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -89,6 +89,7 @@ assert "`maintenance`" in rotation and "`drained` or `disabled`" in rotation checkpoint_match = '"CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=$PEM_DEST"' assert checkpoint_match in rotation +assert "! -path '/var/lib/ci-fleet/checkpoints/.checkpoint.staging.*/*'" in rotation assert "retain the old GitHub key and old PEM" in rotation revocation = app_setup[ From c3d4572d75cae15599207c92f0b499797d706570 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:26:04 -0500 Subject: [PATCH 10/25] docs: harden PEM transfer and cleanup --- docs/GITHUB-APP-SETUP.md | 47 +++++++++++++++++++++++++++++--------- scripts/test_quickstart.py | 23 +++++++++++++++---- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 20558863..5b2fe23a 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -74,8 +74,8 @@ PEM_DEST="/etc/ci-fleet/secrets/github-app-ROTATION-ID.pem" Then run the same transfer and verification sequence for either case. The path validation makes it safe to quote `PEM_DEST` in the remote shell command, the -rotation guard rejects the active path, and shell noclobber prevents replacing -any existing file: +rotation guard rejects the active path, and an atomic hard-link creation refuses +any existing destination node before the key can appear at that path: ```bash valid_pem_path() { @@ -90,17 +90,27 @@ fi PEM_DIR=${PEM_DEST%/*} [[ -n "$PEM_DIR" ]] || PEM_DIR=/ -# PEM_DEST is validated above and intentionally expanded client-side. +# PEM_DIR and PEM_DEST are validated above and intentionally expanded locally. # shellcheck disable=SC2029 if -ssh "$CONTROLLER" \ - '{ test -d "'"$PEM_DIR"'" || install -d -m 0700 "'"$PEM_DIR"'"; } && - umask 077 && set -C && cat > "'"$PEM_DEST"'" && - chown root:root "'"$PEM_DEST"'" && - chmod 0600 "'"$PEM_DEST"'"' <"$PEM" && - local_sha=$(sha256sum -- "$PEM" | cut -d' ' -f1) && - remote_sha=$(ssh "$CONTROLLER" \ - "sha256sum -- \"$PEM_DEST\" | cut -d' ' -f1") && +ssh "$CONTROLLER" " + { test -d \"$PEM_DIR\" || install -d -m 0700 \"$PEM_DIR\"; } && + umask 077 && + tmp=\$(mktemp -- \"$PEM_DIR/.github-app-key.XXXXXX\") && + trap 'rm -f -- \"\$tmp\"' 0 && + cat >\"\$tmp\" && + chown root:root \"\$tmp\" && + chmod 0600 \"\$tmp\" && + ln -- \"\$tmp\" \"$PEM_DEST\" && + rm -f -- \"\$tmp\" && + trap - 0 +" <"$PEM" && + local_sha=$(sha256sum -- "$PEM") && + local_sha=${local_sha%% *} && + [[ "$local_sha" =~ ^[0-9a-f]{64}$ ]] && + remote_sha=$(ssh "$CONTROLLER" "sha256sum -- \"$PEM_DEST\"") && + remote_sha=${remote_sha%% *} && + [[ "$remote_sha" =~ ^[0-9a-f]{64}$ ]] && test "$local_sha" = "$remote_sha" && ssh "$CONTROLLER" \ "test \"\$(stat -c '%U:%G' -- \"$PEM_DEST\")\" = 'root:root'" && @@ -239,6 +249,13 @@ New controller: new app. Do not share one app across controllers. checkpoint references `PEM_DEST`, not `ACTIVE_PEM`: ```bash + valid_pem_path() { + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ && $1 != *//* && + ! $1 =~ (^|/)\.\.?(/|$) ]] + } + PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ + /etc/ci-fleet/host.env | cut -d= -f2-) + valid_pem_path "$PEM_DEST" || exit 1 LATEST_CHECKPOINT=$(sudo find /var/lib/ci-fleet/checkpoints \ -mindepth 2 -maxdepth 2 -type f -name .complete \ ! -path '/var/lib/ci-fleet/checkpoints/.checkpoint.staging.*/*' \ @@ -309,6 +326,14 @@ Retirement is not complete when only the App installation is removed: # Put PEM_DEST and every old path in exactly one array; never use a wildcard. LOCAL_PEMS=("/etc/ci-fleet/secrets/HOST-LOCAL-KEY.pem") MANAGED_PEMS=("/run/secret-manager/MANAGER-BACKED-KEY") + # Retire both a host-local symlink and its backing key file. + for pem in "${LOCAL_PEMS[@]}"; do + if sudo test -L "$pem"; then + backing=$(sudo readlink -f -- "$pem") || exit 1 + valid_pem_path "$backing" || exit 1 + LOCAL_PEMS+=("$backing") + fi + done PEM_INVENTORY=/etc/ci-fleet/retired-pem-paths configured_classifications=0 for pem in "${LOCAL_PEMS[@]}" "${MANAGED_PEMS[@]}"; do diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index ab51211d..f3baa9c2 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -44,21 +44,27 @@ ) ] active_guard = '[[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]' -noclobber_line = next(line for line in transfer.splitlines() if "set -C && cat >" in line) -assert '"$PEM_DEST"' in noclobber_line assert "valid_pem_path \"$PEM_DEST\" || exit 1" in transfer assert '[[ -n "$PEM_DIR" ]] || PEM_DIR=/' in transfer -assert 'test -d "\'"$PEM_DIR"\'" || install -d -m 0700 "\'"$PEM_DIR"\'"' in transfer +assert "install -d -m 0700" in transfer and "$PEM_DIR" in transfer assert "^/[A-Za-z0-9._/-]+$" in transfer assert active_guard in transfer assert transfer.index(active_guard) < transfer.index('ssh "$CONTROLLER"') -for command in ("cat >", "sha256sum --", "stat -c '%U:%G'", "stat -c '%a'"): +assert "mktemp --" in transfer and 'cat >\\"\\$tmp\\"' in transfer +hard_link = 'ln -- \\"\\$tmp\\" \\"$PEM_DEST\\"' +assert hard_link in transfer +assert transfer.index("mktemp --") < transfer.index('cat >\\"\\$tmp\\"') +assert transfer.index('cat >\\"\\$tmp\\"') < transfer.index(hard_link) +assert "set -C" not in transfer +for command in ("sha256sum --", "stat -c '%U:%G'", "stat -c '%a'"): assert any(command in line and "$PEM_DEST" in line for line in transfer.splitlines()), ( f"transfer does not use configured destination: {command}" ) verification = transfer[transfer.index('if\nssh "$CONTROLLER"') : transfer.index("then\n if rm")] for check in ( + '[[ "$local_sha" =~ ^[0-9a-f]{64}$ ]] &&\n', + '[[ "$remote_sha" =~ ^[0-9a-f]{64}$ ]] &&\n', 'test "$local_sha" = "$remote_sha" &&\n', "= 'root:root'\" &&\n", "= '600'\"\n", @@ -89,6 +95,12 @@ assert "`maintenance`" in rotation and "`drained` or `disabled`" in rotation checkpoint_match = '"CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=$PEM_DEST"' assert checkpoint_match in rotation +checkpoint_query = "LATEST_CHECKPOINT=$(sudo find" +checkpoint_destination = "PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE='" +assert rotation.index(checkpoint_destination) < rotation.index(checkpoint_query) +assert rotation.index('valid_pem_path "$PEM_DEST" || exit 1') < rotation.index( + checkpoint_query +) assert "! -path '/var/lib/ci-fleet/checkpoints/.checkpoint.staging.*/*'" in rotation assert "retain the old GitHub key and old PEM" in rotation @@ -118,6 +130,9 @@ manager_absent = 'sudo test ! -e "$pem" || exit 1' inventory_distinct = '[[ "$pem" != "$PEM_INVENTORY" ]] || exit 1' assert 'LOCAL_PEMS=(' in retirement and 'MANAGED_PEMS=(' in retirement +assert 'if sudo test -L "$pem"; then' in retirement +assert 'backing=$(sudo readlink -f -- "$pem") || exit 1' in retirement +assert 'LOCAL_PEMS+=("$backing")' in retirement assert "^/[A-Za-z0-9._/-]+$" in retirement assert retirement.index(read_destination) < retirement.index(validate_paths) assert retirement.index(validate_paths) < retirement.index(inventory_distinct) From ec7bdbcd8859fe998f6fc1e6f01b369ccbf716b3 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:34:47 -0500 Subject: [PATCH 11/25] docs: close PEM path edge cases --- docs/GITHUB-APP-SETUP.md | 17 +++++++++++++---- scripts/test_quickstart.py | 10 ++++++---- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 5b2fe23a..0ffcf3d1 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -101,7 +101,7 @@ ssh "$CONTROLLER" " cat >\"\$tmp\" && chown root:root \"\$tmp\" && chmod 0600 \"\$tmp\" && - ln -- \"\$tmp\" \"$PEM_DEST\" && + ln -T -- \"\$tmp\" \"$PEM_DEST\" && rm -f -- \"\$tmp\" && trap - 0 " <"$PEM" && @@ -290,11 +290,20 @@ Only after every activation check above succeeds: PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem" - for pem in "$PEM_DEST" "$ACTIVE_PEM"; do + OLD_PEM_PATHS=("$ACTIVE_PEM") + if sudo test -L "$ACTIVE_PEM"; then + backing=$(sudo readlink -f -- "$ACTIVE_PEM") || exit 1 + OLD_PEM_PATHS+=("$backing") + fi + valid_pem_path "$PEM_DEST" || exit 1 + for pem in "${OLD_PEM_PATHS[@]}"; do valid_pem_path "$pem" || exit 1 + [[ "$pem" != "$PEM_DEST" ]] || exit 1 + done + for pem in "${OLD_PEM_PATHS[@]}"; do + sudo rm -f -- "$pem" || exit 1 done - [[ "$ACTIVE_PEM" != "$PEM_DEST" ]] || exit 1 - sudo rm -f -- "$ACTIVE_PEM" + unset backing OLD_PEM_PATHS ``` 3. Remove any old workstation or temporary copies under the applicable secure diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index f3baa9c2..5c065440 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -51,7 +51,7 @@ assert active_guard in transfer assert transfer.index(active_guard) < transfer.index('ssh "$CONTROLLER"') assert "mktemp --" in transfer and 'cat >\\"\\$tmp\\"' in transfer -hard_link = 'ln -- \\"\\$tmp\\" \\"$PEM_DEST\\"' +hard_link = 'ln -T -- \\"\\$tmp\\" \\"$PEM_DEST\\"' assert hard_link in transfer assert transfer.index("mktemp --") < transfer.index('cat >\\"\\$tmp\\"') assert transfer.index('cat >\\"\\$tmp\\"') < transfer.index(hard_link) @@ -110,10 +110,12 @@ ) ] read_destination = "PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE='" -validate_paths = 'for pem in "$PEM_DEST" "$ACTIVE_PEM"; do' -distinct_paths = '[[ "$ACTIVE_PEM" != "$PEM_DEST" ]] || exit 1' -remove_old_pem = 'sudo rm -f -- "$ACTIVE_PEM"' +validate_paths = 'for pem in "${OLD_PEM_PATHS[@]}"; do' +distinct_paths = '[[ "$pem" != "$PEM_DEST" ]] || exit 1' +remove_old_pem = 'sudo rm -f -- "$pem" || exit 1' assert 'ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem"' in revocation +assert 'backing=$(sudo readlink -f -- "$ACTIVE_PEM") || exit 1' in revocation +assert 'OLD_PEM_PATHS+=("$backing")' in revocation assert "^/[A-Za-z0-9._/-]+$" in revocation assert revocation.index(read_destination) < revocation.index(validate_paths) assert revocation.index(validate_paths) < revocation.index(distinct_paths) From c5db9918e1e5144a09dda7276811379dcec1482f Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:00:15 -0500 Subject: [PATCH 12/25] docs: fail closed through key revocation --- docs/GITHUB-APP-SETUP.md | 50 ++++++++++++++++++++++++++------------ scripts/test_quickstart.py | 18 +++++++++++--- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 0ffcf3d1..8f4e9cbf 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -192,7 +192,7 @@ sudo /opt/ci-fleet/manager/current/scripts/remote-reconcile.sh \ --check-only --installed-ref ``` -`CHECK_OK` means the app can read and validate the installed desired-state +`RECONCILE CONVERGED` means the app can read and validate the installed desired-state commit and the controller is converged. The reconcile script consumes its token internally; it does not print the token. A 403 or "Repository not found" means the installation lacks the repository or the `contents: read` grant — @@ -226,6 +226,11 @@ New controller: new app. Do not share one app across controllers. --env-file /etc/ci-fleet/host.env >/dev/null ``` + If token generation fails, immediately restore + `CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE` in `host.env` to the exact recorded + `ACTIVE_PEM`, verify token generation with the old key, and run normal + reconciliation. Stop; retain both PEMs and do not revoke the old GitHub key. + 4. Run a normal reconciliation, not `--check-only`. The host-configuration drift forces the installer upgrade path, recreates the controller with the new PEM mount, and runs its post-activation health check: @@ -240,7 +245,7 @@ New controller: new app. Do not share one app across controllers. Stop if reconciliation does not report `RECONCILE_OK`, the health check is not `healthy` for an `active` controller or `maintenance` for a controller whose reviewed desired state is `drained` or `disabled`, or the final check - does not report `CHECK_OK`. Warning and unhealthy results always fail. + does not report `RECONCILE CONVERGED`. Warning and unhealthy results always fail. Restore `CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE` to the exact value of `ACTIVE_PEM` and reconcile again; do not revoke the old key or remove either PEM until @@ -275,12 +280,13 @@ New controller: new app. Do not share one app across controllers. Only after every activation check above succeeds: -1. On the GitHub App settings page, under **Private keys**, delete/revoke the - old key. -2. In the current shell, read the new active destination from `host.env` and +1. In the current shell, read the new active destination from `host.env` and reassign `ACTIVE_PEM` to the exact old path recorded before activation. - Validate both paths and their inequality before removing the old PEM; do not - use a broad wildcard in a shared secrets directory: + Put it in exactly one array: `OLD_LOCAL_PEMS` for host-local storage or + `OLD_MANAGED_PEMS` for secret-manager-backed storage. Remove manager-backed + material through that manager and verify it is absent before running this + block. The block resolves host-local symlinks, validates every exact path, + and removes only host-local material; do not use a wildcard: ```bash valid_pem_path() { @@ -290,22 +296,36 @@ Only after every activation check above succeeds: PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem" - OLD_PEM_PATHS=("$ACTIVE_PEM") - if sudo test -L "$ACTIVE_PEM"; then - backing=$(sudo readlink -f -- "$ACTIVE_PEM") || exit 1 - OLD_PEM_PATHS+=("$backing") - fi + OLD_LOCAL_PEMS=("$ACTIVE_PEM") + OLD_MANAGED_PEMS=() + for pem in "${OLD_LOCAL_PEMS[@]}"; do + if sudo test -L "$pem"; then + backing=$(sudo readlink -f -- "$pem") || exit 1 + valid_pem_path "$backing" || exit 1 + OLD_LOCAL_PEMS+=("$backing") + fi + done valid_pem_path "$PEM_DEST" || exit 1 - for pem in "${OLD_PEM_PATHS[@]}"; do + active_classifications=0 + for pem in "${OLD_LOCAL_PEMS[@]}" "${OLD_MANAGED_PEMS[@]}"; do valid_pem_path "$pem" || exit 1 [[ "$pem" != "$PEM_DEST" ]] || exit 1 + if [[ "$pem" == "$ACTIVE_PEM" ]]; then + active_classifications=$((active_classifications + 1)) + fi + done + ((active_classifications == 1)) || exit 1 + for pem in "${OLD_MANAGED_PEMS[@]}"; do + sudo test ! -e "$pem" || exit 1 done - for pem in "${OLD_PEM_PATHS[@]}"; do + for pem in "${OLD_LOCAL_PEMS[@]}"; do sudo rm -f -- "$pem" || exit 1 done - unset backing OLD_PEM_PATHS + unset active_classifications backing OLD_LOCAL_PEMS OLD_MANAGED_PEMS ``` +2. On the GitHub App settings page, under **Private keys**, delete/revoke the + old key. Do not revoke it unless step 1 completed. 3. Remove any old workstation or temporary copies under the applicable secure erasure policy. Keep the new key and its configured path unchanged. diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 5c065440..5f41f44a 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -93,6 +93,9 @@ ] assert "`healthy` for an `active` controller" in rotation assert "`maintenance`" in rotation and "`drained` or `disabled`" in rotation +assert "If token generation fails, immediately restore" in rotation +assert "run normal\n reconciliation" in rotation +assert "`RECONCILE CONVERGED`" in rotation checkpoint_match = '"CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=$PEM_DEST"' assert checkpoint_match in rotation checkpoint_query = "LATEST_CHECKPOINT=$(sudo find" @@ -110,16 +113,23 @@ ) ] read_destination = "PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE='" -validate_paths = 'for pem in "${OLD_PEM_PATHS[@]}"; do' +validate_paths = 'for pem in "${OLD_LOCAL_PEMS[@]}" "${OLD_MANAGED_PEMS[@]}"; do' distinct_paths = '[[ "$pem" != "$PEM_DEST" ]] || exit 1' remove_old_pem = 'sudo rm -f -- "$pem" || exit 1' assert 'ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem"' in revocation -assert 'backing=$(sudo readlink -f -- "$ACTIVE_PEM") || exit 1' in revocation -assert 'OLD_PEM_PATHS+=("$backing")' in revocation +assert 'backing=$(sudo readlink -f -- "$pem") || exit 1' in revocation +assert 'OLD_LOCAL_PEMS+=("$backing")' in revocation +assert 'OLD_MANAGED_PEMS=()' in revocation +assert "((active_classifications == 1)) || exit 1" in revocation +old_manager_absent = 'for pem in "${OLD_MANAGED_PEMS[@]}"; do' assert "^/[A-Za-z0-9._/-]+$" in revocation assert revocation.index(read_destination) < revocation.index(validate_paths) assert revocation.index(validate_paths) < revocation.index(distinct_paths) -assert revocation.index(distinct_paths) < revocation.index(remove_old_pem) +assert revocation.index(distinct_paths) < revocation.index(old_manager_absent) +assert revocation.index(old_manager_absent) < revocation.index(remove_old_pem) +assert revocation.index(remove_old_pem) < revocation.index( + "On the GitHub App settings page" +) retirement = app_setup[app_setup.index("## Controller retirement and PEM removal") :] validate_paths = 'valid_pem_path "$pem" || exit 1' From 61c16147c382ac36e463237097e8f922ec0dd374 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:17:39 -0500 Subject: [PATCH 13/25] docs: clean failed staged PEMs --- docs/GITHUB-APP-SETUP.md | 20 ++++++++++++++++---- scripts/test_quickstart.py | 10 ++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 8f4e9cbf..0a2df6b0 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -90,6 +90,7 @@ fi PEM_DIR=${PEM_DEST%/*} [[ -n "$PEM_DIR" ]] || PEM_DIR=/ +created_dest=false # PEM_DIR and PEM_DEST are validated above and intentionally expanded locally. # shellcheck disable=SC2029 if @@ -97,14 +98,21 @@ ssh "$CONTROLLER" " { test -d \"$PEM_DIR\" || install -d -m 0700 \"$PEM_DIR\"; } && umask 077 && tmp=\$(mktemp -- \"$PEM_DIR/.github-app-key.XXXXXX\") && - trap 'rm -f -- \"\$tmp\"' 0 && + linked=false && + trap 'status=\$?; rm -f -- \"\$tmp\"; + if [ \"\$status\" -ne 0 ] && [ \"\$linked\" = true ]; then + rm -f -- \"$PEM_DEST\"; + fi; + exit \"\$status\"' 0 && cat >\"\$tmp\" && chown root:root \"\$tmp\" && chmod 0600 \"\$tmp\" && ln -T -- \"\$tmp\" \"$PEM_DEST\" && + linked=true && rm -f -- \"\$tmp\" && trap - 0 " <"$PEM" && + created_dest=true && local_sha=$(sha256sum -- "$PEM") && local_sha=${local_sha%% *} && [[ "$local_sha" =~ ^[0-9a-f]{64}$ ]] && @@ -118,15 +126,19 @@ ssh "$CONTROLLER" " "test \"\$(stat -c '%a' -- \"$PEM_DEST\")\" = '600'" then if rm -f -- "$PEM"; then - unset PEM local_sha remote_sha + unset PEM created_dest local_sha remote_sha else printf 'verified transfer, but could not delete downloaded PEM: %s\n' \ "$PEM" >&2 exit 1 fi else + if $created_dest && ! ssh "$CONTROLLER" "rm -f -- \"$PEM_DEST\""; then + printf 'remote cleanup failed; retained inactive destination: %s\n' \ + "$PEM_DEST" >&2 + fi printf 'transfer verification failed; retained downloaded PEM: %s\n' "$PEM" >&2 - unset local_sha remote_sha + unset created_dest local_sha remote_sha exit 1 fi ``` @@ -302,7 +314,7 @@ Only after every activation check above succeeds: if sudo test -L "$pem"; then backing=$(sudo readlink -f -- "$pem") || exit 1 valid_pem_path "$backing" || exit 1 - OLD_LOCAL_PEMS+=("$backing") + OLD_LOCAL_PEMS=("$backing" "$pem") fi done valid_pem_path "$PEM_DEST" || exit 1 diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 5f41f44a..9df616c3 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -56,6 +56,9 @@ assert transfer.index("mktemp --") < transfer.index('cat >\\"\\$tmp\\"') assert transfer.index('cat >\\"\\$tmp\\"') < transfer.index(hard_link) assert "set -C" not in transfer +assert "created_dest=false" in transfer +assert transfer.index(hard_link) < transfer.index("created_dest=true &&") +assert '[ \\"\\$linked\\" = true ]' in transfer for command in ("sha256sum --", "stat -c '%U:%G'", "stat -c '%a'"): assert any(command in line and "$PEM_DEST" in line for line in transfer.splitlines()), ( f"transfer does not use configured destination: {command}" @@ -73,13 +76,16 @@ delete_download = transfer[ transfer.index('if rm -f -- "$PEM"; then') : transfer.index( - "else\n printf 'transfer verification failed" + "else\n if $created_dest" ) ] delete_success, delete_failure = delete_download.split("else", 1) assert "unset PEM" in delete_success assert '"$PEM" >&2' in delete_failure and "exit 1" in delete_failure assert 'rm -f -- "$PEM"' not in transfer[transfer.index("transfer verification failed") :] +verification_failure = transfer[transfer.index("else\n if $created_dest") :] +assert '! ssh "$CONTROLLER" "rm -f -- \\"$PEM_DEST\\""' in verification_failure +assert "remote cleanup failed; retained inactive destination" in verification_failure for use in ( "exact value of `PEM_DEST`", @@ -118,7 +124,7 @@ remove_old_pem = 'sudo rm -f -- "$pem" || exit 1' assert 'ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem"' in revocation assert 'backing=$(sudo readlink -f -- "$pem") || exit 1' in revocation -assert 'OLD_LOCAL_PEMS+=("$backing")' in revocation +assert 'OLD_LOCAL_PEMS=("$backing" "$pem")' in revocation assert 'OLD_MANAGED_PEMS=()' in revocation assert "((active_classifications == 1)) || exit 1" in revocation old_manager_absent = 'for pem in "${OLD_MANAGED_PEMS[@]}"; do' From 4bf03961241bdbbc7815639b0e5286c24c9d669a Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:34:13 -0500 Subject: [PATCH 14/25] docs: cover managed and canonical PEM paths --- docs/GITHUB-APP-SETUP.md | 74 ++++++++++++++++++++++++++++++-------- scripts/test_quickstart.py | 10 +++++- 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 0a2df6b0..ff26a826 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -72,15 +72,57 @@ ACTIVE_PEM="/etc/ci-fleet/secrets/github-app.pem" PEM_DEST="/etc/ci-fleet/secrets/github-app-ROTATION-ID.pem" ``` -Then run the same transfer and verification sequence for either case. The path +Lifecycle commands use canonical absolute paths so path comparisons and exact +deletion cannot change meaning. Before rotating an existing non-canonical path +(for example one containing `..`, repeated separators, or a symlinked parent), +resolve it with `sudo readlink -f -- "$ACTIVE_PEM"`, update `host.env` to that +canonical result, then reconcile and verify healthy convergence before +continuing. Record the canonical result as `ACTIVE_PEM`. New destinations must +also equal `realpath -m -- "$PEM_DEST"`. + +The SSH workflow below is only for a host-local destination. For a +secret-manager-backed destination, do not write into the materialized mount. +Instead, use that manager's authenticated import/version operation to create a +new inactive version from `$PEM`, materialize it at a distinct canonical +`PEM_DEST`, and run the following verification. Activate that version only in +step 2 of the rotation procedure. If verification fails, retain the download, +remove only the new inactive version through the manager, and leave the active +version and path untouched: + +```bash +# PEM_DEST is canonical and intentionally expanded client-side. +# shellcheck disable=SC2029 +if + local_sha=$(sha256sum -- "$PEM") && + local_sha=${local_sha%% *} && + [[ "$local_sha" =~ ^[0-9a-f]{64}$ ]] && + remote_sha=$(ssh "$CONTROLLER" "sha256sum -- \"$PEM_DEST\"") && + remote_sha=${remote_sha%% *} && + [[ "$remote_sha" =~ ^[0-9a-f]{64}$ ]] && + test "$local_sha" = "$remote_sha" && + ssh "$CONTROLLER" \ + "test \"\$(stat -c '%U:%G' -- \"$PEM_DEST\")\" = 'root:root'" && + ssh "$CONTROLLER" \ + "test \"\$(stat -c '%a' -- \"$PEM_DEST\")\" = '600'" +then + rm -f -- "$PEM" || exit 1 +else + printf 'manager import verification failed; retained download: %s\n' \ + "$PEM" >&2 + exit 1 +fi +``` + +For a host-local destination, run the transfer and verification sequence below. +The path validation makes it safe to quote `PEM_DEST` in the remote shell command, the rotation guard rejects the active path, and an atomic hard-link creation refuses any existing destination node before the key can appear at that path: ```bash valid_pem_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+$ && $1 != *//* && - ! $1 =~ (^|/)\.\.?(/|$) ]] + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ ]] && + [[ $(realpath -m -- "$1") == "$1" ]] } valid_pem_path "$PEM_DEST" || exit 1 if [[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]; then @@ -267,8 +309,8 @@ New controller: new app. Do not share one app across controllers. ```bash valid_pem_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+$ && $1 != *//* && - ! $1 =~ (^|/)\.\.?(/|$) ]] + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ ]] && + [[ $(realpath -m -- "$1") == "$1" ]] } PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) @@ -302,20 +344,21 @@ Only after every activation check above succeeds: ```bash valid_pem_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+$ && $1 != *//* && - ! $1 =~ (^|/)\.\.?(/|$) ]] + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ ]] && + [[ $(realpath -m -- "$1") == "$1" ]] } PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem" OLD_LOCAL_PEMS=("$ACTIVE_PEM") OLD_MANAGED_PEMS=() + PEM_DEST_BACKING=$(sudo readlink -f -- "$PEM_DEST") || exit 1 + valid_pem_path "$PEM_DEST_BACKING" || exit 1 for pem in "${OLD_LOCAL_PEMS[@]}"; do - if sudo test -L "$pem"; then - backing=$(sudo readlink -f -- "$pem") || exit 1 - valid_pem_path "$backing" || exit 1 - OLD_LOCAL_PEMS=("$backing" "$pem") - fi + backing=$(sudo readlink -f -- "$pem") || exit 1 + valid_pem_path "$backing" || exit 1 + [[ "$backing" != "$PEM_DEST_BACKING" ]] || exit 1 + OLD_LOCAL_PEMS=("$backing") done valid_pem_path "$PEM_DEST" || exit 1 active_classifications=0 @@ -333,7 +376,8 @@ Only after every activation check above succeeds: for pem in "${OLD_LOCAL_PEMS[@]}"; do sudo rm -f -- "$pem" || exit 1 done - unset active_classifications backing OLD_LOCAL_PEMS OLD_MANAGED_PEMS + unset active_classifications backing PEM_DEST_BACKING \ + OLD_LOCAL_PEMS OLD_MANAGED_PEMS ``` 2. On the GitHub App settings page, under **Private keys**, delete/revoke the @@ -359,8 +403,8 @@ Retirement is not complete when only the App installation is removed: ```bash valid_pem_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+$ && $1 != *//* && - ! $1 =~ (^|/)\.\.?(/|$) ]] + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ ]] && + [[ $(realpath -m -- "$1") == "$1" ]] } PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 9df616c3..5e2df6a0 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -48,6 +48,7 @@ assert '[[ -n "$PEM_DIR" ]] || PEM_DIR=/' in transfer assert "install -d -m 0700" in transfer and "$PEM_DIR" in transfer assert "^/[A-Za-z0-9._/-]+$" in transfer +assert '[[ $(realpath -m -- "$1") == "$1" ]]' in transfer assert active_guard in transfer assert transfer.index(active_guard) < transfer.index('ssh "$CONTROLLER"') assert "mktemp --" in transfer and 'cat >\\"\\$tmp\\"' in transfer @@ -86,6 +87,11 @@ verification_failure = transfer[transfer.index("else\n if $created_dest") :] assert '! ssh "$CONTROLLER" "rm -f -- \\"$PEM_DEST\\""' in verification_failure assert "remote cleanup failed; retained inactive destination" in verification_failure +assert "secret-manager-backed destination" in app_setup +assert "authenticated import/version operation" in app_setup +assert "remove only the new inactive version through the manager" in app_setup +assert "sudo readlink -f -- \"$ACTIVE_PEM\"" in app_setup +assert "update `host.env` to that\ncanonical result" in app_setup for use in ( "exact value of `PEM_DEST`", @@ -123,8 +129,10 @@ distinct_paths = '[[ "$pem" != "$PEM_DEST" ]] || exit 1' remove_old_pem = 'sudo rm -f -- "$pem" || exit 1' assert 'ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem"' in revocation +assert 'PEM_DEST_BACKING=$(sudo readlink -f -- "$PEM_DEST") || exit 1' in revocation assert 'backing=$(sudo readlink -f -- "$pem") || exit 1' in revocation -assert 'OLD_LOCAL_PEMS=("$backing" "$pem")' in revocation +assert '[[ "$backing" != "$PEM_DEST_BACKING" ]] || exit 1' in revocation +assert 'OLD_LOCAL_PEMS=("$backing")' in revocation assert 'OLD_MANAGED_PEMS=()' in revocation assert "((active_classifications == 1)) || exit 1" in revocation old_manager_absent = 'for pem in "${OLD_MANAGED_PEMS[@]}"; do' From 92c4a1f05a26ae60d1801c55b4711ae6a8ef2815 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:47:17 -0500 Subject: [PATCH 15/25] docs: canonicalize PEM lifecycle remotely --- docs/GITHUB-APP-SETUP.md | 24 ++++++++++++++---------- scripts/test_quickstart.py | 10 +++++----- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index ff26a826..45b16ae7 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -73,7 +73,8 @@ PEM_DEST="/etc/ci-fleet/secrets/github-app-ROTATION-ID.pem" ``` Lifecycle commands use canonical absolute paths so path comparisons and exact -deletion cannot change meaning. Before rotating an existing non-canonical path +deletion cannot change meaning. Before rotating, revoking, or directly retiring +an existing non-canonical path (for example one containing `..`, repeated separators, or a symlinked parent), resolve it with `sudo readlink -f -- "$ACTIVE_PEM"`, update `host.env` to that canonical result, then reconcile and verify healthy convergence before @@ -93,6 +94,8 @@ version and path untouched: # PEM_DEST is canonical and intentionally expanded client-side. # shellcheck disable=SC2029 if + ssh "$CONTROLLER" \ + "test \"\$(realpath -m -- \"$PEM_DEST\")\" = \"$PEM_DEST\"" && local_sha=$(sha256sum -- "$PEM") && local_sha=${local_sha%% *} && [[ "$local_sha" =~ ^[0-9a-f]{64}$ ]] && @@ -121,8 +124,7 @@ any existing destination node before the key can appear at that path: ```bash valid_pem_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+$ ]] && - [[ $(realpath -m -- "$1") == "$1" ]] + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ ]] } valid_pem_path "$PEM_DEST" || exit 1 if [[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]; then @@ -132,10 +134,11 @@ fi PEM_DIR=${PEM_DEST%/*} [[ -n "$PEM_DIR" ]] || PEM_DIR=/ -created_dest=false # PEM_DIR and PEM_DEST are validated above and intentionally expanded locally. # shellcheck disable=SC2029 if +ssh "$CONTROLLER" \ + "test \"\$(realpath -m -- \"$PEM_DEST\")\" = \"$PEM_DEST\"" && ssh "$CONTROLLER" " { test -d \"$PEM_DIR\" || install -d -m 0700 \"$PEM_DIR\"; } && umask 077 && @@ -154,7 +157,6 @@ ssh "$CONTROLLER" " rm -f -- \"\$tmp\" && trap - 0 " <"$PEM" && - created_dest=true && local_sha=$(sha256sum -- "$PEM") && local_sha=${local_sha%% *} && [[ "$local_sha" =~ ^[0-9a-f]{64}$ ]] && @@ -168,27 +170,29 @@ ssh "$CONTROLLER" " "test \"\$(stat -c '%a' -- \"$PEM_DEST\")\" = '600'" then if rm -f -- "$PEM"; then - unset PEM created_dest local_sha remote_sha + unset PEM local_sha remote_sha else printf 'verified transfer, but could not delete downloaded PEM: %s\n' \ "$PEM" >&2 exit 1 fi else - if $created_dest && ! ssh "$CONTROLLER" "rm -f -- \"$PEM_DEST\""; then + if ! ssh "$CONTROLLER" "rm -f -- \"$PEM_DEST\""; then printf 'remote cleanup failed; retained inactive destination: %s\n' \ "$PEM_DEST" >&2 fi printf 'transfer verification failed; retained downloaded PEM: %s\n' "$PEM" >&2 - unset created_dest local_sha remote_sha + unset local_sha remote_sha exit 1 fi ``` Use an equivalent privileged SSH workflow if direct root login is disabled. If transfer, checksum, owner, or mode verification fails, the sequence stops, -leaves the active controller PEM untouched, and retains the downloaded -replacement for diagnosis or a safe retry. Do not revoke the old GitHub key. +removes the exact destination declared inactive even if SSH returned an +ambiguous result, leaves the active controller PEM untouched, and retains the +downloaded replacement for diagnosis or a safe retry. Do not revoke the old +GitHub key. After success, remove any other copy from the browser download location, trash, sync, and temporary storage according to the workstation's secure-erasure policy; plain `rm` may not erase data from snapshots, SSDs, or copy-on-write diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 5e2df6a0..425429e8 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -48,7 +48,7 @@ assert '[[ -n "$PEM_DIR" ]] || PEM_DIR=/' in transfer assert "install -d -m 0700" in transfer and "$PEM_DIR" in transfer assert "^/[A-Za-z0-9._/-]+$" in transfer -assert '[[ $(realpath -m -- "$1") == "$1" ]]' in transfer +assert 'realpath -m -- \\"$PEM_DEST\\"' in transfer assert active_guard in transfer assert transfer.index(active_guard) < transfer.index('ssh "$CONTROLLER"') assert "mktemp --" in transfer and 'cat >\\"\\$tmp\\"' in transfer @@ -57,8 +57,6 @@ assert transfer.index("mktemp --") < transfer.index('cat >\\"\\$tmp\\"') assert transfer.index('cat >\\"\\$tmp\\"') < transfer.index(hard_link) assert "set -C" not in transfer -assert "created_dest=false" in transfer -assert transfer.index(hard_link) < transfer.index("created_dest=true &&") assert '[ \\"\\$linked\\" = true ]' in transfer for command in ("sha256sum --", "stat -c '%U:%G'", "stat -c '%a'"): assert any(command in line and "$PEM_DEST" in line for line in transfer.splitlines()), ( @@ -77,21 +75,23 @@ delete_download = transfer[ transfer.index('if rm -f -- "$PEM"; then') : transfer.index( - "else\n if $created_dest" + "else\n if ! ssh" ) ] delete_success, delete_failure = delete_download.split("else", 1) assert "unset PEM" in delete_success assert '"$PEM" >&2' in delete_failure and "exit 1" in delete_failure assert 'rm -f -- "$PEM"' not in transfer[transfer.index("transfer verification failed") :] -verification_failure = transfer[transfer.index("else\n if $created_dest") :] +verification_failure = transfer[transfer.index("else\n if ! ssh") :] assert '! ssh "$CONTROLLER" "rm -f -- \\"$PEM_DEST\\""' in verification_failure assert "remote cleanup failed; retained inactive destination" in verification_failure +assert "even if SSH returned an\nambiguous result" in app_setup assert "secret-manager-backed destination" in app_setup assert "authenticated import/version operation" in app_setup assert "remove only the new inactive version through the manager" in app_setup assert "sudo readlink -f -- \"$ACTIVE_PEM\"" in app_setup assert "update `host.env` to that\ncanonical result" in app_setup +assert "rotating, revoking, or directly retiring" in app_setup for use in ( "exact value of `PEM_DEST`", From dea1294983eeec9db3f3777602efdab24a2dd587 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:00:08 -0500 Subject: [PATCH 16/25] docs: identify staged PEM ownership --- docs/GITHUB-APP-SETUP.md | 41 +++++++++++++++++++++++++------------- scripts/test_quickstart.py | 13 +++++++----- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 45b16ae7..2a5d0d97 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -133,6 +133,10 @@ if [[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]; then fi PEM_DIR=${PEM_DEST%/*} [[ -n "$PEM_DIR" ]] || PEM_DIR=/ +TRANSFER_ID=$(< /proc/sys/kernel/random/uuid) || exit 1 +PEM_MARKER="$PEM_DEST.ci-fleet-transfer-$TRANSFER_ID" +[[ $PEM_MARKER =~ ^/[A-Za-z0-9._/-]+$ ]] || exit 1 +[[ -z "$ACTIVE_PEM" || "$PEM_MARKER" != "$ACTIVE_PEM" ]] || exit 1 # PEM_DIR and PEM_DEST are validated above and intentionally expanded locally. # shellcheck disable=SC2029 @@ -143,17 +147,20 @@ ssh "$CONTROLLER" " { test -d \"$PEM_DIR\" || install -d -m 0700 \"$PEM_DIR\"; } && umask 077 && tmp=\$(mktemp -- \"$PEM_DIR/.github-app-key.XXXXXX\") && - linked=false && trap 'status=\$?; rm -f -- \"\$tmp\"; - if [ \"\$status\" -ne 0 ] && [ \"\$linked\" = true ]; then - rm -f -- \"$PEM_DEST\"; + if [ \"\$status\" -ne 0 ] && + [ -e \"$PEM_MARKER\" ] && [ \"$PEM_MARKER\" -ef \"\$tmp\" ]; then + if [ -e \"$PEM_DEST\" ] && [ \"$PEM_DEST\" -ef \"$PEM_MARKER\" ]; then + rm -f -- \"$PEM_DEST\"; + fi; + rm -f -- \"$PEM_MARKER\"; fi; exit \"\$status\"' 0 && cat >\"\$tmp\" && chown root:root \"\$tmp\" && chmod 0600 \"\$tmp\" && + ln -T -- \"\$tmp\" \"$PEM_MARKER\" && ln -T -- \"\$tmp\" \"$PEM_DEST\" && - linked=true && rm -f -- \"\$tmp\" && trap - 0 " <"$PEM" && @@ -167,32 +174,38 @@ ssh "$CONTROLLER" " ssh "$CONTROLLER" \ "test \"\$(stat -c '%U:%G' -- \"$PEM_DEST\")\" = 'root:root'" && ssh "$CONTROLLER" \ - "test \"\$(stat -c '%a' -- \"$PEM_DEST\")\" = '600'" + "test \"\$(stat -c '%a' -- \"$PEM_DEST\")\" = '600'" && + ssh "$CONTROLLER" \ + "test \"$PEM_MARKER\" -ef \"$PEM_DEST\" && rm -f -- \"$PEM_MARKER\"" then if rm -f -- "$PEM"; then - unset PEM local_sha remote_sha + unset PEM local_sha remote_sha TRANSFER_ID PEM_MARKER else printf 'verified transfer, but could not delete downloaded PEM: %s\n' \ "$PEM" >&2 exit 1 fi else - if ! ssh "$CONTROLLER" "rm -f -- \"$PEM_DEST\""; then - printf 'remote cleanup failed; retained inactive destination: %s\n' \ + if ! ssh "$CONTROLLER" \ + "if test -e \"$PEM_MARKER\" && test \"$PEM_MARKER\" -ef \"$PEM_DEST\"; then + rm -f -- \"$PEM_DEST\" \"$PEM_MARKER\"; + elif test -e \"$PEM_MARKER\"; then exit 1; fi"; then + printf 'remote ownership cleanup failed; inspect inactive destination: %s\n' \ "$PEM_DEST" >&2 fi printf 'transfer verification failed; retained downloaded PEM: %s\n' "$PEM" >&2 - unset local_sha remote_sha + unset local_sha remote_sha TRANSFER_ID PEM_MARKER exit 1 fi ``` Use an equivalent privileged SSH workflow if direct root login is disabled. -If transfer, checksum, owner, or mode verification fails, the sequence stops, -removes the exact destination declared inactive even if SSH returned an -ambiguous result, leaves the active controller PEM untouched, and retains the -downloaded replacement for diagnosis or a safe retry. Do not revoke the old -GitHub key. +If transfer, checksum, owner, or mode verification fails, the sequence stops. +It removes `PEM_DEST` only when the per-transfer hard-link marker proves that +this invocation created that inode, including after an ambiguous SSH result; +pre-existing destinations are preserved. The active controller PEM remains +untouched and the downloaded replacement is retained for diagnosis or a safe +retry. Do not revoke the old GitHub key. After success, remove any other copy from the browser download location, trash, sync, and temporary storage according to the workstation's secure-erasure policy; plain `rm` may not erase data from snapshots, SSDs, or copy-on-write diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 425429e8..2cbe75e8 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -57,7 +57,8 @@ assert transfer.index("mktemp --") < transfer.index('cat >\\"\\$tmp\\"') assert transfer.index('cat >\\"\\$tmp\\"') < transfer.index(hard_link) assert "set -C" not in transfer -assert '[ \\"\\$linked\\" = true ]' in transfer +marker_link = 'ln -T -- \\"\\$tmp\\" \\"$PEM_MARKER\\"' +assert transfer.index(marker_link) < transfer.index(hard_link) for command in ("sha256sum --", "stat -c '%U:%G'", "stat -c '%a'"): assert any(command in line and "$PEM_DEST" in line for line in transfer.splitlines()), ( f"transfer does not use configured destination: {command}" @@ -69,7 +70,7 @@ '[[ "$remote_sha" =~ ^[0-9a-f]{64}$ ]] &&\n', 'test "$local_sha" = "$remote_sha" &&\n', "= 'root:root'\" &&\n", - "= '600'\"\n", + "= '600'\" &&\n", ): assert check in verification, f"download deletion is not gated by: {check}" @@ -83,9 +84,11 @@ assert '"$PEM" >&2' in delete_failure and "exit 1" in delete_failure assert 'rm -f -- "$PEM"' not in transfer[transfer.index("transfer verification failed") :] verification_failure = transfer[transfer.index("else\n if ! ssh") :] -assert '! ssh "$CONTROLLER" "rm -f -- \\"$PEM_DEST\\""' in verification_failure -assert "remote cleanup failed; retained inactive destination" in verification_failure -assert "even if SSH returned an\nambiguous result" in app_setup +assert 'test \\"$PEM_MARKER\\" -ef \\"$PEM_DEST\\"' in verification_failure +assert 'rm -f -- \\"$PEM_DEST\\" \\"$PEM_MARKER\\"' in verification_failure +assert "remote ownership cleanup failed" in verification_failure +assert "per-transfer hard-link marker proves" in app_setup +assert "pre-existing destinations are preserved" in app_setup assert "secret-manager-backed destination" in app_setup assert "authenticated import/version operation" in app_setup assert "remove only the new inactive version through the manager" in app_setup From defb74690176a6c048a45760f09ace21d4b6c9bf Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:09:22 -0500 Subject: [PATCH 17/25] docs: clean transfer markers before temp unlink --- docs/GITHUB-APP-SETUP.md | 3 ++- scripts/test_quickstart.py | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 2a5d0d97..fc2ddd44 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -147,7 +147,7 @@ ssh "$CONTROLLER" " { test -d \"$PEM_DIR\" || install -d -m 0700 \"$PEM_DIR\"; } && umask 077 && tmp=\$(mktemp -- \"$PEM_DIR/.github-app-key.XXXXXX\") && - trap 'status=\$?; rm -f -- \"\$tmp\"; + trap 'status=\$?; if [ \"\$status\" -ne 0 ] && [ -e \"$PEM_MARKER\" ] && [ \"$PEM_MARKER\" -ef \"\$tmp\" ]; then if [ -e \"$PEM_DEST\" ] && [ \"$PEM_DEST\" -ef \"$PEM_MARKER\" ]; then @@ -155,6 +155,7 @@ ssh "$CONTROLLER" " fi; rm -f -- \"$PEM_MARKER\"; fi; + rm -f -- \"\$tmp\"; exit \"\$status\"' 0 && cat >\"\$tmp\" && chown root:root \"\$tmp\" && diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 2cbe75e8..0e6d6c54 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -59,6 +59,10 @@ assert "set -C" not in transfer marker_link = 'ln -T -- \\"\\$tmp\\" \\"$PEM_MARKER\\"' assert transfer.index(marker_link) < transfer.index(hard_link) +trap_body = transfer[transfer.index("trap 'status=") : transfer.index("exit \\\"\\$status\\\"' 0")] +assert trap_body.index('\\"$PEM_MARKER\\" -ef \\"\\$tmp\\"') < trap_body.index( + 'rm -f -- \\"\\$tmp\\"' +) for command in ("sha256sum --", "stat -c '%U:%G'", "stat -c '%a'"): assert any(command in line and "$PEM_DEST" in line for line in transfer.splitlines()), ( f"transfer does not use configured destination: {command}" From 18fd8210ecccae440a1d9f819504c813d125144b Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:24:11 -0500 Subject: [PATCH 18/25] docs: make marker cleanup retry-safe --- docs/GITHUB-APP-SETUP.md | 28 ++++++++++++++++++++-------- scripts/test_quickstart.py | 14 +++++++++++--- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index fc2ddd44..ae3b3218 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -177,8 +177,16 @@ ssh "$CONTROLLER" " ssh "$CONTROLLER" \ "test \"\$(stat -c '%a' -- \"$PEM_DEST\")\" = '600'" && ssh "$CONTROLLER" \ - "test \"$PEM_MARKER\" -ef \"$PEM_DEST\" && rm -f -- \"$PEM_MARKER\"" + "test \"$PEM_MARKER\" -ef \"$PEM_DEST\"" then + if ! ssh "$CONTROLLER" \ + "if test -e \"$PEM_MARKER\"; then + test \"$PEM_MARKER\" -ef \"$PEM_DEST\" && rm -f -- \"$PEM_MARKER\"; + fi"; then + printf 'verified transfer; retry idempotent marker cleanup before activation: %s\n' \ + "$PEM_MARKER" >&2 + exit 1 + fi if rm -f -- "$PEM"; then unset PEM local_sha remote_sha TRANSFER_ID PEM_MARKER else @@ -206,7 +214,9 @@ It removes `PEM_DEST` only when the per-transfer hard-link marker proves that this invocation created that inode, including after an ambiguous SSH result; pre-existing destinations are preserved. The active controller PEM remains untouched and the downloaded replacement is retained for diagnosis or a safe -retry. Do not revoke the old GitHub key. +retry. If marker cleanup itself returns an ambiguous SSH result, rerun only its +idempotent cleanup command before activation; do not rerun the transfer. Do not +revoke the old GitHub key. After success, remove any other copy from the browser download location, trash, sync, and temporary storage according to the workstation's secure-erasure policy; plain `rm` may not erase data from snapshots, SSDs, or copy-on-write @@ -420,12 +430,13 @@ Retirement is not complete when only the App installation is removed: preserves `/etc/ci-fleet/host.env` and `/etc/ci-fleet/secrets`: ```bash - valid_pem_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+$ ]] && - [[ $(realpath -m -- "$1") == "$1" ]] + safe_pem_path() { + [[ $1 =~ ^/[A-Za-z0-9._/-]+$ ]] } PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) + safe_pem_path "$PEM_DEST" || exit 1 + [[ $(realpath -m -- "$PEM_DEST") == "$PEM_DEST" ]] || exit 1 # Put PEM_DEST and every old path in exactly one array; never use a wildcard. LOCAL_PEMS=("/etc/ci-fleet/secrets/HOST-LOCAL-KEY.pem") MANAGED_PEMS=("/run/secret-manager/MANAGER-BACKED-KEY") @@ -433,15 +444,16 @@ Retirement is not complete when only the App installation is removed: for pem in "${LOCAL_PEMS[@]}"; do if sudo test -L "$pem"; then backing=$(sudo readlink -f -- "$pem") || exit 1 - valid_pem_path "$backing" || exit 1 + safe_pem_path "$backing" || exit 1 LOCAL_PEMS+=("$backing") fi done PEM_INVENTORY=/etc/ci-fleet/retired-pem-paths configured_classifications=0 for pem in "${LOCAL_PEMS[@]}" "${MANAGED_PEMS[@]}"; do - valid_pem_path "$pem" || exit 1 - [[ "$pem" != "$PEM_INVENTORY" ]] || exit 1 + safe_pem_path "$pem" || exit 1 + [[ $(realpath -m -- "$pem") != \ + $(realpath -m -- "$PEM_INVENTORY") ]] || exit 1 if [[ "$pem" == "$PEM_DEST" ]]; then configured_classifications=$((configured_classifications + 1)) fi diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 0e6d6c54..2aeee7f5 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -68,7 +68,8 @@ f"transfer does not use configured destination: {command}" ) -verification = transfer[transfer.index('if\nssh "$CONTROLLER"') : transfer.index("then\n if rm")] +main_then = transfer.index("\nthen\n if ! ssh", transfer.index("stat -c '%a'")) +verification = transfer[transfer.index('if\nssh "$CONTROLLER"') : main_then] for check in ( '[[ "$local_sha" =~ ^[0-9a-f]{64}$ ]] &&\n', '[[ "$remote_sha" =~ ^[0-9a-f]{64}$ ]] &&\n', @@ -93,6 +94,12 @@ assert "remote ownership cleanup failed" in verification_failure assert "per-transfer hard-link marker proves" in app_setup assert "pre-existing destinations are preserved" in app_setup +verification_ack = transfer[transfer.index("stat -c '%a'") : main_then] +assert 'test \\"$PEM_MARKER\\" -ef \\"$PEM_DEST\\"' in verification_ack +assert 'rm -f -- \\"$PEM_MARKER\\"' not in verification_ack +marker_cleanup = transfer[main_then : transfer.index('if rm -f -- "$PEM"')] +assert 'if test -e \\"$PEM_MARKER\\"; then' in marker_cleanup +assert "retry idempotent marker cleanup before activation" in marker_cleanup assert "secret-manager-backed destination" in app_setup assert "authenticated import/version operation" in app_setup assert "remove only the new inactive version through the manager" in app_setup @@ -153,7 +160,7 @@ ) retirement = app_setup[app_setup.index("## Controller retirement and PEM removal") :] -validate_paths = 'valid_pem_path "$pem" || exit 1' +validate_paths = 'safe_pem_path "$pem" || exit 1' uninstall = "scripts/install-worker-controller.sh \\\n --uninstall &&" remove_pem = 'sudo rm -f -- "$pem" || return 1' persist_inventory = 'sudo install -m 0600 /dev/stdin "$PEM_INVENTORY"' @@ -161,12 +168,13 @@ remove_inventory = 'sudo rm -f -- "$PEM_INVENTORY"; then' classify_destination = 'configured_classifications=$((configured_classifications + 1))' manager_absent = 'sudo test ! -e "$pem" || exit 1' -inventory_distinct = '[[ "$pem" != "$PEM_INVENTORY" ]] || exit 1' +inventory_distinct = '$(realpath -m -- "$PEM_INVENTORY") ]] || exit 1' assert 'LOCAL_PEMS=(' in retirement and 'MANAGED_PEMS=(' in retirement assert 'if sudo test -L "$pem"; then' in retirement assert 'backing=$(sudo readlink -f -- "$pem") || exit 1' in retirement assert 'LOCAL_PEMS+=("$backing")' in retirement assert "^/[A-Za-z0-9._/-]+$" in retirement +assert '[[ $(realpath -m -- "$PEM_DEST") == "$PEM_DEST" ]] || exit 1' in retirement assert retirement.index(read_destination) < retirement.index(validate_paths) assert retirement.index(validate_paths) < retirement.index(inventory_distinct) assert retirement.index(inventory_distinct) < retirement.index(classify_destination) From 5144ca7c7d517318ad96b58c5110a54c42e5ae5a Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:34:57 -0500 Subject: [PATCH 19/25] docs: bound transfer marker filenames --- docs/GITHUB-APP-SETUP.md | 2 +- scripts/test_quickstart.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index ae3b3218..9984f6f1 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -134,7 +134,7 @@ fi PEM_DIR=${PEM_DEST%/*} [[ -n "$PEM_DIR" ]] || PEM_DIR=/ TRANSFER_ID=$(< /proc/sys/kernel/random/uuid) || exit 1 -PEM_MARKER="$PEM_DEST.ci-fleet-transfer-$TRANSFER_ID" +PEM_MARKER="$PEM_DIR/.ci-fleet-transfer-$TRANSFER_ID" [[ $PEM_MARKER =~ ^/[A-Za-z0-9._/-]+$ ]] || exit 1 [[ -z "$ACTIVE_PEM" || "$PEM_MARKER" != "$ACTIVE_PEM" ]] || exit 1 diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 2aeee7f5..d919e79c 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -46,6 +46,8 @@ active_guard = '[[ -n "$ACTIVE_PEM" && "$PEM_DEST" == "$ACTIVE_PEM" ]]' assert "valid_pem_path \"$PEM_DEST\" || exit 1" in transfer assert '[[ -n "$PEM_DIR" ]] || PEM_DIR=/' in transfer +assert 'PEM_MARKER="$PEM_DIR/.ci-fleet-transfer-$TRANSFER_ID"' in transfer +assert 'PEM_MARKER="$PEM_DEST' not in transfer assert "install -d -m 0700" in transfer and "$PEM_DIR" in transfer assert "^/[A-Za-z0-9._/-]+$" in transfer assert 'realpath -m -- \\"$PEM_DEST\\"' in transfer From b760079c9d2634de998ef3b2525630238cf653ec Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:50:24 -0500 Subject: [PATCH 20/25] docs: preserve symlink classification contracts --- docs/GITHUB-APP-SETUP.md | 13 ++++++++----- scripts/test_quickstart.py | 17 +++++++++++------ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 9984f6f1..bfd51473 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -380,6 +380,14 @@ Only after every activation check above succeeds: ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem" OLD_LOCAL_PEMS=("$ACTIVE_PEM") OLD_MANAGED_PEMS=() + active_classifications=0 + for pem in "${OLD_LOCAL_PEMS[@]}" "${OLD_MANAGED_PEMS[@]}"; do + [[ $pem =~ ^/[A-Za-z0-9._/-]+$ ]] || exit 1 + if [[ "$pem" == "$ACTIVE_PEM" ]]; then + active_classifications=$((active_classifications + 1)) + fi + done + ((active_classifications == 1)) || exit 1 PEM_DEST_BACKING=$(sudo readlink -f -- "$PEM_DEST") || exit 1 valid_pem_path "$PEM_DEST_BACKING" || exit 1 for pem in "${OLD_LOCAL_PEMS[@]}"; do @@ -389,15 +397,10 @@ Only after every activation check above succeeds: OLD_LOCAL_PEMS=("$backing") done valid_pem_path "$PEM_DEST" || exit 1 - active_classifications=0 for pem in "${OLD_LOCAL_PEMS[@]}" "${OLD_MANAGED_PEMS[@]}"; do valid_pem_path "$pem" || exit 1 [[ "$pem" != "$PEM_DEST" ]] || exit 1 - if [[ "$pem" == "$ACTIVE_PEM" ]]; then - active_classifications=$((active_classifications + 1)) - fi done - ((active_classifications == 1)) || exit 1 for pem in "${OLD_MANAGED_PEMS[@]}"; do sudo test ! -e "$pem" || exit 1 done diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index d919e79c..6f1cb5a8 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -20,13 +20,15 @@ assert "managed controller managed controller" not in quickstart app_setup = (repo_root / "docs" / "GITHUB-APP-SETUP.md").read_text() -token_calls = app_setup.count("scripts/github-app-token.sh \\") -redirected_token_calls = re.findall( - r"scripts/github-app-token\.sh \\\n\s+--env-file [^\n]+ >/dev/null", - app_setup, +bash_blocks = "\n".join(re.findall(r"```bash\n(.*?)\n\s*```", app_setup, re.S)) +bash_commands = re.sub(r"\\\n\s*", " ", bash_blocks).splitlines() +token_commands = [ + command for command in bash_commands if "scripts/github-app-token.sh" in command +] +assert len(token_commands) == 2, f"expected two documented token-helper calls, found {len(token_commands)}" +assert all(">/dev/null" in command for command in token_commands), ( + "token-helper stdout must be redirected" ) -assert token_calls == 2, f"expected two documented token-helper calls, found {token_calls}" -assert len(redirected_token_calls) == token_calls, "token-helper stdout must be redirected" assert app_setup.index("## Key rotation: activate and verify before revocation") < app_setup.index( "## Old-key revocation" ) @@ -151,6 +153,9 @@ assert 'OLD_LOCAL_PEMS=("$backing")' in revocation assert 'OLD_MANAGED_PEMS=()' in revocation assert "((active_classifications == 1)) || exit 1" in revocation +assert revocation.index("((active_classifications == 1)) || exit 1") < revocation.index( + 'OLD_LOCAL_PEMS=("$backing")' +) old_manager_absent = 'for pem in "${OLD_MANAGED_PEMS[@]}"; do' assert "^/[A-Za-z0-9._/-]+$" in revocation assert revocation.index(read_destination) < revocation.index(validate_paths) From d41085c9f261b2dc50022cb32661cbc7b8e91a59 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 21:04:15 -0500 Subject: [PATCH 21/25] docs: preserve complete privileged PEM inventories --- docs/GITHUB-APP-SETUP.md | 20 +++++++++++++------- scripts/test_quickstart.py | 21 ++++++++++++++++----- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index bfd51473..79412ec8 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -94,6 +94,7 @@ version and path untouched: # PEM_DEST is canonical and intentionally expanded client-side. # shellcheck disable=SC2029 if + [[ $PEM_DEST =~ ^/[A-Za-z0-9._/-]+$ ]] && ssh "$CONTROLLER" \ "test \"\$(realpath -m -- \"$PEM_DEST\")\" = \"$PEM_DEST\"" && local_sha=$(sha256sum -- "$PEM") && @@ -390,16 +391,20 @@ Only after every activation check above succeeds: ((active_classifications == 1)) || exit 1 PEM_DEST_BACKING=$(sudo readlink -f -- "$PEM_DEST") || exit 1 valid_pem_path "$PEM_DEST_BACKING" || exit 1 + RESOLVED_OLD_LOCAL_PEMS=() for pem in "${OLD_LOCAL_PEMS[@]}"; do backing=$(sudo readlink -f -- "$pem") || exit 1 valid_pem_path "$backing" || exit 1 [[ "$backing" != "$PEM_DEST_BACKING" ]] || exit 1 - OLD_LOCAL_PEMS=("$backing") + RESOLVED_OLD_LOCAL_PEMS+=("$backing") + [[ "$pem" == "$backing" ]] || RESOLVED_OLD_LOCAL_PEMS+=("$pem") done + OLD_LOCAL_PEMS=("${RESOLVED_OLD_LOCAL_PEMS[@]}") valid_pem_path "$PEM_DEST" || exit 1 for pem in "${OLD_LOCAL_PEMS[@]}" "${OLD_MANAGED_PEMS[@]}"; do - valid_pem_path "$pem" || exit 1 - [[ "$pem" != "$PEM_DEST" ]] || exit 1 + [[ $pem =~ ^/[A-Za-z0-9._/-]+$ ]] || exit 1 + pem_backing=$(sudo realpath -m -- "$pem") || exit 1 + [[ "$pem_backing" != "$PEM_DEST_BACKING" ]] || exit 1 done for pem in "${OLD_MANAGED_PEMS[@]}"; do sudo test ! -e "$pem" || exit 1 @@ -407,7 +412,8 @@ Only after every activation check above succeeds: for pem in "${OLD_LOCAL_PEMS[@]}"; do sudo rm -f -- "$pem" || exit 1 done - unset active_classifications backing PEM_DEST_BACKING \ + unset active_classifications backing pem_backing PEM_DEST_BACKING \ + RESOLVED_OLD_LOCAL_PEMS \ OLD_LOCAL_PEMS OLD_MANAGED_PEMS ``` @@ -439,7 +445,7 @@ Retirement is not complete when only the App installation is removed: PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=' \ /etc/ci-fleet/host.env | cut -d= -f2-) safe_pem_path "$PEM_DEST" || exit 1 - [[ $(realpath -m -- "$PEM_DEST") == "$PEM_DEST" ]] || exit 1 + [[ $(sudo realpath -m -- "$PEM_DEST") == "$PEM_DEST" ]] || exit 1 # Put PEM_DEST and every old path in exactly one array; never use a wildcard. LOCAL_PEMS=("/etc/ci-fleet/secrets/HOST-LOCAL-KEY.pem") MANAGED_PEMS=("/run/secret-manager/MANAGER-BACKED-KEY") @@ -455,8 +461,8 @@ Retirement is not complete when only the App installation is removed: configured_classifications=0 for pem in "${LOCAL_PEMS[@]}" "${MANAGED_PEMS[@]}"; do safe_pem_path "$pem" || exit 1 - [[ $(realpath -m -- "$pem") != \ - $(realpath -m -- "$PEM_INVENTORY") ]] || exit 1 + [[ $(sudo realpath -m -- "$pem") != \ + $(sudo realpath -m -- "$PEM_INVENTORY") ]] || exit 1 if [[ "$pem" == "$PEM_DEST" ]]; then configured_classifications=$((configured_classifications + 1)) fi diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 6f1cb5a8..23d23d30 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -107,6 +107,14 @@ assert "secret-manager-backed destination" in app_setup assert "authenticated import/version operation" in app_setup assert "remove only the new inactive version through the manager" in app_setup +manager_workflow = app_setup[ + app_setup.index("secret-manager-backed destination, do not") : app_setup.index( + "For a host-local destination" + ) +] +assert manager_workflow.index('[[ $PEM_DEST =~ ^/[A-Za-z0-9._/-]+$ ]]') < manager_workflow.index( + 'ssh "$CONTROLLER"' +) assert "sudo readlink -f -- \"$ACTIVE_PEM\"" in app_setup assert "update `host.env` to that\ncanonical result" in app_setup assert "rotating, revoking, or directly retiring" in app_setup @@ -144,17 +152,19 @@ ] read_destination = "PEM_DEST=$(sudo grep -E '^CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE='" validate_paths = 'for pem in "${OLD_LOCAL_PEMS[@]}" "${OLD_MANAGED_PEMS[@]}"; do' -distinct_paths = '[[ "$pem" != "$PEM_DEST" ]] || exit 1' +distinct_paths = '[[ "$pem_backing" != "$PEM_DEST_BACKING" ]] || exit 1' remove_old_pem = 'sudo rm -f -- "$pem" || exit 1' assert 'ACTIVE_PEM="/etc/ci-fleet/secrets/OLD-GITHUB-APP-KEY.pem"' in revocation assert 'PEM_DEST_BACKING=$(sudo readlink -f -- "$PEM_DEST") || exit 1' in revocation assert 'backing=$(sudo readlink -f -- "$pem") || exit 1' in revocation assert '[[ "$backing" != "$PEM_DEST_BACKING" ]] || exit 1' in revocation -assert 'OLD_LOCAL_PEMS=("$backing")' in revocation +assert 'RESOLVED_OLD_LOCAL_PEMS+=("$backing")' in revocation +assert 'RESOLVED_OLD_LOCAL_PEMS+=("$pem")' in revocation +assert 'OLD_LOCAL_PEMS=("${RESOLVED_OLD_LOCAL_PEMS[@]}")' in revocation assert 'OLD_MANAGED_PEMS=()' in revocation assert "((active_classifications == 1)) || exit 1" in revocation assert revocation.index("((active_classifications == 1)) || exit 1") < revocation.index( - 'OLD_LOCAL_PEMS=("$backing")' + 'OLD_LOCAL_PEMS=("${RESOLVED_OLD_LOCAL_PEMS[@]}")' ) old_manager_absent = 'for pem in "${OLD_MANAGED_PEMS[@]}"; do' assert "^/[A-Za-z0-9._/-]+$" in revocation @@ -175,13 +185,14 @@ remove_inventory = 'sudo rm -f -- "$PEM_INVENTORY"; then' classify_destination = 'configured_classifications=$((configured_classifications + 1))' manager_absent = 'sudo test ! -e "$pem" || exit 1' -inventory_distinct = '$(realpath -m -- "$PEM_INVENTORY") ]] || exit 1' +inventory_distinct = '$(sudo realpath -m -- "$PEM_INVENTORY") ]] || exit 1' assert 'LOCAL_PEMS=(' in retirement and 'MANAGED_PEMS=(' in retirement assert 'if sudo test -L "$pem"; then' in retirement assert 'backing=$(sudo readlink -f -- "$pem") || exit 1' in retirement assert 'LOCAL_PEMS+=("$backing")' in retirement assert "^/[A-Za-z0-9._/-]+$" in retirement -assert '[[ $(realpath -m -- "$PEM_DEST") == "$PEM_DEST" ]] || exit 1' in retirement +assert '[[ $(sudo realpath -m -- "$PEM_DEST") == "$PEM_DEST" ]] || exit 1' in retirement +assert '$(sudo realpath -m -- "$PEM_INVENTORY")' in retirement assert retirement.index(read_destination) < retirement.index(validate_paths) assert retirement.index(validate_paths) < retirement.index(inventory_distinct) assert retirement.index(inventory_distinct) < retirement.index(classify_destination) From bbddb39bc03e095fb223bc469f89bc1d2d67e3dd Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 21:21:46 -0500 Subject: [PATCH 22/25] docs: complete PEM safety preflight --- docs/GITHUB-APP-SETUP.md | 50 +++++++++++++++++++++++++++++--------- scripts/test_quickstart.py | 14 ++++++++--- 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 79412ec8..dd969d5e 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -78,8 +78,31 @@ an existing non-canonical path (for example one containing `..`, repeated separators, or a symlinked parent), resolve it with `sudo readlink -f -- "$ACTIVE_PEM"`, update `host.env` to that canonical result, then reconcile and verify healthy convergence before -continuing. Record the canonical result as `ACTIVE_PEM`. New destinations must -also equal `realpath -m -- "$PEM_DEST"`. +continuing. Record the canonical path as `ACTIVE_PEM`. + +Before either transfer workflow, reject the active path and key itself as the +replacement. This preflight runs before importing a manager-backed version: + +```bash +[[ $PEM_DEST =~ ^/[A-Za-z0-9._/-]+$ ]] || exit 1 +if [[ -n "$ACTIVE_PEM" ]]; then + [[ $ACTIVE_PEM =~ ^/[A-Za-z0-9._/-]+$ ]] || exit 1 + [[ "$PEM_DEST" != "$ACTIVE_PEM" ]] || exit 1 + set -o pipefail + replacement_pubkey_sha=$(openssl pkey -in "$PEM" -pubout -outform DER | \ + sha256sum) || exit 1 + replacement_pubkey_sha=${replacement_pubkey_sha%% *} + # ACTIVE_PEM is shell-safe above and intentionally expanded client-side. + # shellcheck disable=SC2029 + active_pubkey_sha=$(ssh "$CONTROLLER" \ + "openssl pkey -in \"$ACTIVE_PEM\" -pubout -outform DER | sha256sum") || exit 1 + active_pubkey_sha=${active_pubkey_sha%% *} + [[ "$replacement_pubkey_sha" =~ ^[0-9a-f]{64}$ ]] || exit 1 + [[ "$active_pubkey_sha" =~ ^[0-9a-f]{64}$ ]] || exit 1 + [[ "$replacement_pubkey_sha" != "$active_pubkey_sha" ]] || exit 1 + unset active_pubkey_sha replacement_pubkey_sha +fi +``` The SSH workflow below is only for a host-local destination. For a secret-manager-backed destination, do not write into the materialized mount. @@ -146,6 +169,8 @@ ssh "$CONTROLLER" \ "test \"\$(realpath -m -- \"$PEM_DEST\")\" = \"$PEM_DEST\"" && ssh "$CONTROLLER" " { test -d \"$PEM_DIR\" || install -d -m 0700 \"$PEM_DIR\"; } && + test \"\$(stat -c '%U' -- \"$PEM_DIR\")\" = root && + test -z \"\$(find \"$PEM_DIR\" -maxdepth 0 -perm /022 -print -quit)\" && umask 077 && tmp=\$(mktemp -- \"$PEM_DIR/.github-app-key.XXXXXX\") && trap 'status=\$?; @@ -199,7 +224,7 @@ else if ! ssh "$CONTROLLER" \ "if test -e \"$PEM_MARKER\" && test \"$PEM_MARKER\" -ef \"$PEM_DEST\"; then rm -f -- \"$PEM_DEST\" \"$PEM_MARKER\"; - elif test -e \"$PEM_MARKER\"; then exit 1; fi"; then + elif test -e \"$PEM_MARKER\"; then rm -f -- \"$PEM_MARKER\"; fi"; then printf 'remote ownership cleanup failed; inspect inactive destination: %s\n' \ "$PEM_DEST" >&2 fi @@ -449,14 +474,15 @@ Retirement is not complete when only the App installation is removed: # Put PEM_DEST and every old path in exactly one array; never use a wildcard. LOCAL_PEMS=("/etc/ci-fleet/secrets/HOST-LOCAL-KEY.pem") MANAGED_PEMS=("/run/secret-manager/MANAGER-BACKED-KEY") - # Retire both a host-local symlink and its backing key file. + # Resolve every host-local path before persisting the retry inventory. + RESOLVED_LOCAL_PEMS=() for pem in "${LOCAL_PEMS[@]}"; do - if sudo test -L "$pem"; then - backing=$(sudo readlink -f -- "$pem") || exit 1 - safe_pem_path "$backing" || exit 1 - LOCAL_PEMS+=("$backing") - fi + backing=$(sudo readlink -f -- "$pem") || exit 1 + safe_pem_path "$backing" || exit 1 + RESOLVED_LOCAL_PEMS+=("$backing") + [[ "$pem" == "$backing" ]] || RESOLVED_LOCAL_PEMS+=("$pem") done + LOCAL_PEMS=("${RESOLVED_LOCAL_PEMS[@]}") PEM_INVENTORY=/etc/ci-fleet/retired-pem-paths configured_classifications=0 for pem in "${LOCAL_PEMS[@]}" "${MANAGED_PEMS[@]}"; do @@ -473,9 +499,9 @@ Retirement is not complete when only the App installation is removed: done if ((${#LOCAL_PEMS[@]})); then printf '%s\n' "${LOCAL_PEMS[@]}" | \ - sudo install -m 0600 /dev/stdin "$PEM_INVENTORY" + sudo install -T -m 0600 /dev/stdin "$PEM_INVENTORY" else - sudo install -m 0600 /dev/null "$PEM_INVENTORY" + sudo install -T -m 0600 /dev/null "$PEM_INVENTORY" fi || exit 1 remove_retired_pems() { @@ -488,7 +514,7 @@ Retirement is not complete when only the App installation is removed: remove_retired_pems && sudo rm -f -- /etc/ci-fleet/host.env && sudo rm -f -- "$PEM_INVENTORY"; then - unset PEM_DEST PEM_INVENTORY LOCAL_PEMS MANAGED_PEMS + unset PEM_DEST PEM_INVENTORY LOCAL_PEMS MANAGED_PEMS RESOLVED_LOCAL_PEMS else printf 'retirement cleanup failed; retained %s and host.env\n' \ "$PEM_INVENTORY" >&2 diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 23d23d30..ed6aa86f 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -51,6 +51,7 @@ assert 'PEM_MARKER="$PEM_DIR/.ci-fleet-transfer-$TRANSFER_ID"' in transfer assert 'PEM_MARKER="$PEM_DEST' not in transfer assert "install -d -m 0700" in transfer and "$PEM_DIR" in transfer +assert "stat -c '%U'" in transfer and "-perm /022" in transfer assert "^/[A-Za-z0-9._/-]+$" in transfer assert 'realpath -m -- \\"$PEM_DEST\\"' in transfer assert active_guard in transfer @@ -96,6 +97,7 @@ assert 'test \\"$PEM_MARKER\\" -ef \\"$PEM_DEST\\"' in verification_failure assert 'rm -f -- \\"$PEM_DEST\\" \\"$PEM_MARKER\\"' in verification_failure assert "remote ownership cleanup failed" in verification_failure +assert 'elif test -e \\"$PEM_MARKER\\"; then rm -f -- \\"$PEM_MARKER\\"' in verification_failure assert "per-transfer hard-link marker proves" in app_setup assert "pre-existing destinations are preserved" in app_setup verification_ack = transfer[transfer.index("stat -c '%a'") : main_then] @@ -107,6 +109,11 @@ assert "secret-manager-backed destination" in app_setup assert "authenticated import/version operation" in app_setup assert "remove only the new inactive version through the manager" in app_setup +manager_import = app_setup.index("authenticated import/version operation") +preflight_guard = '[[ "$PEM_DEST" != "$ACTIVE_PEM" ]] || exit 1' +assert app_setup.index(preflight_guard) < manager_import +assert app_setup.index('replacement_pubkey_sha=$(openssl pkey') < manager_import +assert '[[ "$replacement_pubkey_sha" != "$active_pubkey_sha" ]] || exit 1' in app_setup manager_workflow = app_setup[ app_setup.index("secret-manager-backed destination, do not") : app_setup.index( "For a host-local destination" @@ -180,16 +187,17 @@ validate_paths = 'safe_pem_path "$pem" || exit 1' uninstall = "scripts/install-worker-controller.sh \\\n --uninstall &&" remove_pem = 'sudo rm -f -- "$pem" || return 1' -persist_inventory = 'sudo install -m 0600 /dev/stdin "$PEM_INVENTORY"' +persist_inventory = 'sudo install -T -m 0600 /dev/stdin "$PEM_INVENTORY"' remove_host_env = "sudo rm -f -- /etc/ci-fleet/host.env &&" remove_inventory = 'sudo rm -f -- "$PEM_INVENTORY"; then' classify_destination = 'configured_classifications=$((configured_classifications + 1))' manager_absent = 'sudo test ! -e "$pem" || exit 1' inventory_distinct = '$(sudo realpath -m -- "$PEM_INVENTORY") ]] || exit 1' assert 'LOCAL_PEMS=(' in retirement and 'MANAGED_PEMS=(' in retirement -assert 'if sudo test -L "$pem"; then' in retirement assert 'backing=$(sudo readlink -f -- "$pem") || exit 1' in retirement -assert 'LOCAL_PEMS+=("$backing")' in retirement +assert 'RESOLVED_LOCAL_PEMS+=("$backing")' in retirement +assert 'RESOLVED_LOCAL_PEMS+=("$pem")' in retirement +assert 'LOCAL_PEMS=("${RESOLVED_LOCAL_PEMS[@]}")' in retirement assert "^/[A-Za-z0-9._/-]+$" in retirement assert '[[ $(sudo realpath -m -- "$PEM_DEST") == "$PEM_DEST" ]] || exit 1' in retirement assert '$(sudo realpath -m -- "$PEM_INVENTORY")' in retirement From 972f44b85eea3d5e137386d3eff24dab96c48722 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 21:37:36 -0500 Subject: [PATCH 23/25] docs: fail closed across PEM preflight --- docs/GITHUB-APP-SETUP.md | 34 +++++++++++++++++++++++----------- scripts/test_quickstart.py | 16 +++++++++++++++- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index dd969d5e..7eb9885d 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -85,23 +85,24 @@ replacement. This preflight runs before importing a manager-backed version: ```bash [[ $PEM_DEST =~ ^/[A-Za-z0-9._/-]+$ ]] || exit 1 +set -o pipefail +replacement_pubkey_sha=$(openssl pkey -in "$PEM" -pubout -outform DER | \ + sha256sum) || exit 1 +replacement_pubkey_sha=${replacement_pubkey_sha%% *} +[[ "$replacement_pubkey_sha" =~ ^[0-9a-f]{64}$ ]] || exit 1 if [[ -n "$ACTIVE_PEM" ]]; then [[ $ACTIVE_PEM =~ ^/[A-Za-z0-9._/-]+$ ]] || exit 1 [[ "$PEM_DEST" != "$ACTIVE_PEM" ]] || exit 1 - set -o pipefail - replacement_pubkey_sha=$(openssl pkey -in "$PEM" -pubout -outform DER | \ - sha256sum) || exit 1 - replacement_pubkey_sha=${replacement_pubkey_sha%% *} # ACTIVE_PEM is shell-safe above and intentionally expanded client-side. # shellcheck disable=SC2029 active_pubkey_sha=$(ssh "$CONTROLLER" \ - "openssl pkey -in \"$ACTIVE_PEM\" -pubout -outform DER | sha256sum") || exit 1 + "bash -o pipefail -c 'openssl pkey -in \"$ACTIVE_PEM\" -pubout -outform DER | sha256sum'") || exit 1 active_pubkey_sha=${active_pubkey_sha%% *} - [[ "$replacement_pubkey_sha" =~ ^[0-9a-f]{64}$ ]] || exit 1 [[ "$active_pubkey_sha" =~ ^[0-9a-f]{64}$ ]] || exit 1 [[ "$replacement_pubkey_sha" != "$active_pubkey_sha" ]] || exit 1 - unset active_pubkey_sha replacement_pubkey_sha + unset active_pubkey_sha fi +unset replacement_pubkey_sha ``` The SSH workflow below is only for a host-local destination. For a @@ -168,9 +169,18 @@ if ssh "$CONTROLLER" \ "test \"\$(realpath -m -- \"$PEM_DEST\")\" = \"$PEM_DEST\"" && ssh "$CONTROLLER" " + secure_pem_ancestors() { + dir=\"$PEM_DIR\"; + while :; do + test \"\$(stat -c '%U' -- \"\$dir\")\" = root || return 1; + test -z \"\$(find \"\$dir\" -maxdepth 0 -perm /022 -print -quit)\" || return 1; + test \"\$dir\" != / || break; + dir=\${dir%/*}; + test -n \"\$dir\" || dir=/; + done; + } { test -d \"$PEM_DIR\" || install -d -m 0700 \"$PEM_DIR\"; } && - test \"\$(stat -c '%U' -- \"$PEM_DIR\")\" = root && - test -z \"\$(find \"$PEM_DIR\" -maxdepth 0 -perm /022 -print -quit)\" && + secure_pem_ancestors && umask 077 && tmp=\$(mktemp -- \"$PEM_DIR/.github-app-key.XXXXXX\") && trap 'status=\$?; @@ -225,8 +235,10 @@ else "if test -e \"$PEM_MARKER\" && test \"$PEM_MARKER\" -ef \"$PEM_DEST\"; then rm -f -- \"$PEM_DEST\" \"$PEM_MARKER\"; elif test -e \"$PEM_MARKER\"; then rm -f -- \"$PEM_MARKER\"; fi"; then - printf 'remote ownership cleanup failed; inspect inactive destination: %s\n' \ - "$PEM_DEST" >&2 + printf 'remote ownership cleanup failed; retain and retry marker %s for destination %s\n' \ + "$PEM_MARKER" "$PEM_DEST" >&2 + unset local_sha remote_sha TRANSFER_ID + exit 1 fi printf 'transfer verification failed; retained downloaded PEM: %s\n' "$PEM" >&2 unset local_sha remote_sha TRANSFER_ID PEM_MARKER diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index ed6aa86f..9aa1e5ed 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -51,7 +51,9 @@ assert 'PEM_MARKER="$PEM_DIR/.ci-fleet-transfer-$TRANSFER_ID"' in transfer assert 'PEM_MARKER="$PEM_DEST' not in transfer assert "install -d -m 0700" in transfer and "$PEM_DIR" in transfer +assert "secure_pem_ancestors()" in transfer assert "stat -c '%U'" in transfer and "-perm /022" in transfer +assert "dir=\\${dir%/*}" in transfer assert "^/[A-Za-z0-9._/-]+$" in transfer assert 'realpath -m -- \\"$PEM_DEST\\"' in transfer assert active_guard in transfer @@ -96,7 +98,14 @@ verification_failure = transfer[transfer.index("else\n if ! ssh") :] assert 'test \\"$PEM_MARKER\\" -ef \\"$PEM_DEST\\"' in verification_failure assert 'rm -f -- \\"$PEM_DEST\\" \\"$PEM_MARKER\\"' in verification_failure -assert "remote ownership cleanup failed" in verification_failure +assert "remote ownership cleanup failed; retain and retry marker" in verification_failure +assert '"$PEM_MARKER" "$PEM_DEST" >&2' in verification_failure +cleanup_failure = verification_failure[ + verification_failure.index("remote ownership cleanup failed") : + verification_failure.index("transfer verification failed") +] +assert "unset local_sha remote_sha TRANSFER_ID\n" in cleanup_failure +assert "PEM_MARKER" not in cleanup_failure.split("unset", 1)[1] assert 'elif test -e \\"$PEM_MARKER\\"; then rm -f -- \\"$PEM_MARKER\\"' in verification_failure assert "per-transfer hard-link marker proves" in app_setup assert "pre-existing destinations are preserved" in app_setup @@ -114,6 +123,11 @@ assert app_setup.index(preflight_guard) < manager_import assert app_setup.index('replacement_pubkey_sha=$(openssl pkey') < manager_import assert '[[ "$replacement_pubkey_sha" != "$active_pubkey_sha" ]] || exit 1' in app_setup +preflight = app_setup[app_setup.index("Before either transfer workflow") : manager_import] +assert preflight.index('replacement_pubkey_sha=$(openssl pkey') < preflight.index( + 'if [[ -n "$ACTIVE_PEM" ]]' +) +assert "bash -o pipefail -c 'openssl pkey" in preflight manager_workflow = app_setup[ app_setup.index("secret-manager-backed destination, do not") : app_setup.index( "For a host-local destination" From df65e90d1dab6122617c0d4221322ecb7519fad3 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 29 Jul 2026 21:51:57 -0500 Subject: [PATCH 24/25] docs: bind PEM verification to activation --- docs/GITHUB-APP-SETUP.md | 31 +++++++++++++++++++++++++------ scripts/test_quickstart.py | 8 ++++++-- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index 7eb9885d..f3389a2d 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -76,9 +76,17 @@ Lifecycle commands use canonical absolute paths so path comparisons and exact deletion cannot change meaning. Before rotating, revoking, or directly retiring an existing non-canonical path (for example one containing `..`, repeated separators, or a symlinked parent), -resolve it with `sudo readlink -f -- "$ACTIVE_PEM"`, update `host.env` to that -canonical result, then reconcile and verify healthy convergence before -continuing. Record the canonical path as `ACTIVE_PEM`. +validate it as shell-safe and resolve it on the controller—not the workstation: + +```bash +[[ $ACTIVE_PEM =~ ^/[A-Za-z0-9._/-]+$ ]] || exit 1 +# ACTIVE_PEM is shell-safe above and intentionally expanded client-side. +# shellcheck disable=SC2029 +ACTIVE_PEM=$(ssh "$CONTROLLER" "readlink -f -- \"$ACTIVE_PEM\"") || exit 1 +``` + +Update controller `host.env` to that canonical result, then reconcile and verify +healthy convergence before continuing. Record the result as `ACTIVE_PEM`. Before either transfer workflow, reject the active path and key itself as the replacement. This preflight runs before importing a manager-backed version: @@ -102,7 +110,6 @@ if [[ -n "$ACTIVE_PEM" ]]; then [[ "$replacement_pubkey_sha" != "$active_pubkey_sha" ]] || exit 1 unset active_pubkey_sha fi -unset replacement_pubkey_sha ``` The SSH workflow below is only for a host-local destination. For a @@ -128,12 +135,18 @@ if remote_sha=${remote_sha%% *} && [[ "$remote_sha" =~ ^[0-9a-f]{64}$ ]] && test "$local_sha" = "$remote_sha" && + remote_pubkey_sha=$(ssh "$CONTROLLER" \ + "bash -o pipefail -c 'openssl pkey -in \"$PEM_DEST\" -pubout -outform DER | sha256sum'") && + remote_pubkey_sha=${remote_pubkey_sha%% *} && + [[ "$remote_pubkey_sha" =~ ^[0-9a-f]{64}$ ]] && + test "$replacement_pubkey_sha" = "$remote_pubkey_sha" && ssh "$CONTROLLER" \ "test \"\$(stat -c '%U:%G' -- \"$PEM_DEST\")\" = 'root:root'" && ssh "$CONTROLLER" \ "test \"\$(stat -c '%a' -- \"$PEM_DEST\")\" = '600'" then rm -f -- "$PEM" || exit 1 + unset replacement_pubkey_sha remote_pubkey_sha else printf 'manager import verification failed; retained download: %s\n' \ "$PEM" >&2 @@ -208,6 +221,11 @@ ssh "$CONTROLLER" " remote_sha=${remote_sha%% *} && [[ "$remote_sha" =~ ^[0-9a-f]{64}$ ]] && test "$local_sha" = "$remote_sha" && + remote_pubkey_sha=$(ssh "$CONTROLLER" \ + "bash -o pipefail -c 'openssl pkey -in \"$PEM_DEST\" -pubout -outform DER | sha256sum'") && + remote_pubkey_sha=${remote_pubkey_sha%% *} && + [[ "$remote_pubkey_sha" =~ ^[0-9a-f]{64}$ ]] && + test "$replacement_pubkey_sha" = "$remote_pubkey_sha" && ssh "$CONTROLLER" \ "test \"\$(stat -c '%U:%G' -- \"$PEM_DEST\")\" = 'root:root'" && ssh "$CONTROLLER" \ @@ -224,7 +242,8 @@ then exit 1 fi if rm -f -- "$PEM"; then - unset PEM local_sha remote_sha TRANSFER_ID PEM_MARKER + unset PEM local_sha remote_sha remote_pubkey_sha replacement_pubkey_sha \ + TRANSFER_ID PEM_MARKER else printf 'verified transfer, but could not delete downloaded PEM: %s\n' \ "$PEM" >&2 @@ -357,7 +376,7 @@ New controller: new app. Do not share one app across controllers. ```bash sudo /opt/ci-fleet/manager/current/scripts/remote-reconcile.sh - sudo /opt/ci-fleet/current/scripts/healthcheck.sh + sudo /opt/ci-fleet/current/scripts/healthcheck.sh || exit 1 sudo /opt/ci-fleet/manager/current/scripts/remote-reconcile.sh \ --check-only --installed-ref ``` diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 9aa1e5ed..5b50d6e1 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -128,6 +128,8 @@ 'if [[ -n "$ACTIVE_PEM" ]]' ) assert "bash -o pipefail -c 'openssl pkey" in preflight +assert "unset replacement_pubkey_sha" not in preflight +assert app_setup.count('test "$replacement_pubkey_sha" = "$remote_pubkey_sha"') == 2 manager_workflow = app_setup[ app_setup.index("secret-manager-backed destination, do not") : app_setup.index( "For a host-local destination" @@ -136,8 +138,8 @@ assert manager_workflow.index('[[ $PEM_DEST =~ ^/[A-Za-z0-9._/-]+$ ]]') < manager_workflow.index( 'ssh "$CONTROLLER"' ) -assert "sudo readlink -f -- \"$ACTIVE_PEM\"" in app_setup -assert "update `host.env` to that\ncanonical result" in app_setup +assert 'ACTIVE_PEM=$(ssh "$CONTROLLER" "readlink -f -- \\"$ACTIVE_PEM\\"")' in app_setup +assert "Update controller `host.env` to that canonical result" in app_setup assert "rotating, revoking, or directly retiring" in app_setup for use in ( @@ -155,6 +157,8 @@ assert "If token generation fails, immediately restore" in rotation assert "run normal\n reconciliation" in rotation assert "`RECONCILE CONVERGED`" in rotation +health_gate = "sudo /opt/ci-fleet/current/scripts/healthcheck.sh || exit 1" +assert rotation.index(health_gate) < rotation.index("--check-only --installed-ref") checkpoint_match = '"CI_FLEET_GITHUB_APP_PRIVATE_KEY_FILE=$PEM_DEST"' assert checkpoint_match in rotation checkpoint_query = "LATEST_CHECKPOINT=$(sudo find" From 5bf859235d0125f70a0a1e0958b252e5a9fc4f47 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Thu, 30 Jul 2026 00:42:43 -0500 Subject: [PATCH 25/25] docs: validate managed PEM ancestors --- docs/GITHUB-APP-SETUP.md | 18 ++++++++++++++++++ scripts/test_quickstart.py | 22 ++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/docs/GITHUB-APP-SETUP.md b/docs/GITHUB-APP-SETUP.md index f3389a2d..5605470a 100644 --- a/docs/GITHUB-APP-SETUP.md +++ b/docs/GITHUB-APP-SETUP.md @@ -128,6 +128,21 @@ if [[ $PEM_DEST =~ ^/[A-Za-z0-9._/-]+$ ]] && ssh "$CONTROLLER" \ "test \"\$(realpath -m -- \"$PEM_DEST\")\" = \"$PEM_DEST\"" && + ssh "$CONTROLLER" " + secure_pem_ancestors() { + dir=\"${PEM_DEST%/*}\"; + test -n \"\$dir\" || dir=/; + while :; do + test \"\$(stat -c '%U' -- \"\$dir\")\" = root || return 1; + test -z \"\$(find \"\$dir\" -maxdepth 0 -perm /022 -print -quit)\" || return 1; + test \"\$dir\" != / || break; + dir=\${dir%/*}; + test -n \"\$dir\" || dir=/; + done; + } + test \"\$(stat -c '%F' -- \"$PEM_DEST\")\" = 'regular file' && + secure_pem_ancestors + " && local_sha=$(sha256sum -- "$PEM") && local_sha=${local_sha%% *} && [[ "$local_sha" =~ ^[0-9a-f]{64}$ ]] && @@ -154,6 +169,9 @@ else fi ``` +Manager layouts whose materialized file or any containing directory fails +these checks are unsupported until secured. + For a host-local destination, run the transfer and verification sequence below. The path validation makes it safe to quote `PEM_DEST` in the remote shell command, the diff --git a/scripts/test_quickstart.py b/scripts/test_quickstart.py index 5b50d6e1..de74c0c2 100644 --- a/scripts/test_quickstart.py +++ b/scripts/test_quickstart.py @@ -138,6 +138,28 @@ assert manager_workflow.index('[[ $PEM_DEST =~ ^/[A-Za-z0-9._/-]+$ ]]') < manager_workflow.index( 'ssh "$CONTROLLER"' ) +manager_condition = manager_workflow[ + manager_workflow.index("if\n") : manager_workflow.index("\nthen\n") +] +manager_ancestor_gate = manager_condition.index("secure_pem_ancestors\n") +regular_file_gate = ( + "test \\\"\\$(stat -c '%F' -- \\\"$PEM_DEST\\\")\\\" = 'regular file' &&" +) +assert regular_file_gate in manager_condition +assert "stat -c '%U'" in manager_condition and "-perm /022" in manager_condition +assert ( + "dir=\\${dir%/*}" in manager_condition + and 'test \\\"\\$dir\\\" != / || break' in manager_condition +) +assert manager_ancestor_gate < manager_condition.index('local_sha=$(sha256sum -- "$PEM")') +assert manager_workflow.index("secure_pem_ancestors\n") < manager_workflow.index('rm -f -- "$PEM"') +manager_failure = manager_workflow[manager_workflow.index("else\n") :] +assert "manager import verification failed" in manager_failure and "exit 1" in manager_failure +assert "unsupported until secured" in manager_workflow +manager_gate = app_setup.index("secure_pem_ancestors\n", manager_import) +rotation_start = app_setup.index("## Key rotation: activate and verify before revocation") +rotation_token = app_setup.index("scripts/github-app-token.sh", rotation_start) +assert manager_gate < rotation_start < rotation_token assert 'ACTIVE_PEM=$(ssh "$CONTROLLER" "readlink -f -- \\"$ACTIVE_PEM\\"")' in app_setup assert "Update controller `host.env` to that canonical result" in app_setup assert "rotating, revoking, or directly retiring" in app_setup