From 9e1cfa5fa80374d2c63c79be072f2ad80280bbcb Mon Sep 17 00:00:00 2001 From: Jens Tirsvad Nielsen Date: Mon, 19 Oct 2026 12:01:00 +0800 Subject: [PATCH 1/5] Fix the review findings in connect-wireguard The wireguard-go choice now survives sudo (it is passed through env), sudo is used only when the job is not root, a configuration field with a line break is refused, only the key values of a full configuration are masked, the key file is removed when wg-quick fails, and the guide calls @main until the release. Refs #184 Task: MIL-012#3 --- .../connect-wireguard/connect-wireguard.sh | 51 +++++++++++++++---- guides/vpn-wireguard.md | 26 ++++++---- tests/test_connect_wireguard.py | 37 +++++++++++++- 3 files changed, 92 insertions(+), 22 deletions(-) diff --git a/actions/connect-wireguard/connect-wireguard.sh b/actions/connect-wireguard/connect-wireguard.sh index 1579fa9..97768a1 100644 --- a/actions/connect-wireguard/connect-wireguard.sh +++ b/actions/connect-wireguard/connect-wireguard.sh @@ -15,7 +15,17 @@ WG_DIR="${WG_DIR:-/etc/wireguard}" WG_QUICK="${WG_QUICK:-wg-quick}" WG_PING="${WG_PING:-ping}" -SUDO="${WG_SUDO-sudo}" +# sudo is used only when the job is not root: a job container runs as root and +# often has no sudo. WG_SUDO overrides it (empty for none). +if [ -n "${WG_SUDO+x}" ]; then + SUDO="$WG_SUDO" +elif [ "$(id -u)" = "0" ]; then + SUDO="" +else + SUDO="sudo" +fi +# variables that must reach wg-quick through sudo, which resets the environment +WG_QUICK_ENV=() fail() { echo "::error::[$1] $2" @@ -23,14 +33,24 @@ fail() { } mask_secrets() { - local value line + local value line key for value in "${WG_PRIVATE_KEY:-}" "${WG_PRESHARED_KEY:-}"; do [ -n "$value" ] && echo "::add-mask::$value" done - # every line of a full configuration is masked, so a key cannot leak in pieces + # a full configuration: mask the value of every PrivateKey and PresharedKey + # line, and nothing else, so the rest of the log stays readable while IFS= read -r line; do - line="${line#"${line%%[![:space:]]*}"}" - [ -n "$line" ] && echo "::add-mask::$line" + line="${line//$' '/}" + key="${line%%=*}" + key="${key//[[:space:]]/}" + case "${key,,}" in + privatekey | presharedkey) + value="${line#*=}" + value="${value#"${value%%[![:space:]]*}"}" + value="${value%"${value##*[![:space:]]}"}" + [ -n "$value" ] && echo "::add-mask::$value" + ;; + esac done <<<"${WG_CONFIG:-}" return 0 } @@ -51,6 +71,11 @@ render_config() { [ -n "${!name:-}" ] || fail configuration "set config, or all of private-key, address, peer-public-key, endpoint and allowed-ips ($name is empty)." done + for name in WG_PRIVATE_KEY WG_ADDRESS WG_PEER_PUBLIC_KEY WG_ENDPOINT WG_ALLOWED_IPS WG_PRESHARED_KEY WG_DNS; do + # a line break would add directives (PostUp runs as root) to the file + [[ "${!name:-}" != *[[:cntrl:]]* ]] || + fail configuration "$name must be one line without control characters." + done [[ "$WG_KEEPALIVE" =~ ^[0-9]+$ ]] || fail configuration "persistent-keepalive must be a number of seconds." printf '[Interface]\nPrivateKey = %s\nAddress = %s\n' "$WG_PRIVATE_KEY" "$WG_ADDRESS" @@ -77,7 +102,7 @@ choose_implementation() { return 0 fi if command -v wireguard-go >/dev/null 2>&1 || $SUDO apt-get install -y -qq wireguard-go 2>/dev/null; then - export WG_QUICK_USERSPACE_IMPLEMENTATION=wireguard-go + WG_QUICK_ENV=(WG_QUICK_USERSPACE_IMPLEMENTATION=wireguard-go) echo "No kernel WireGuard module; using wireguard-go." else fail environment "the runner has no WireGuard kernel module and wireguard-go could not be installed; the runner container needs NET_ADMIN and /dev/net/tun (see guides/vpn-wireguard.md)." @@ -97,8 +122,12 @@ tunnel_up() { $SUDO install -d -m 700 "$WG_DIR" (umask 077 && printf '%s\n' "$rendered" | $SUDO tee "$conf" >/dev/null) $SUDO chmod 600 "$conf" - $SUDO "$WG_QUICK" up "$WG_INTERFACE" || + # env keeps the userspace variable through sudo; the file holds the key, so + # it is removed when the interface does not come up + if ! $SUDO env ${WG_QUICK_ENV[@]+"${WG_QUICK_ENV[@]}"} "$WG_QUICK" up "$WG_INTERFACE"; then + $SUDO rm -f "$conf" fail connection "wg-quick could not bring $WG_INTERFACE up." + fi if [ -n "${WG_HEALTH_CHECK_HOST:-}" ]; then "$WG_PING" -c 3 -W 5 "$WG_HEALTH_CHECK_HOST" >/dev/null 2>&1 || fail connection "health-check-host '$WG_HEALTH_CHECK_HOST' does not answer through the tunnel." @@ -119,10 +148,10 @@ main() { WG_INTERFACE="${WG_INTERFACE:-wg0}" WG_KEEPALIVE="${WG_KEEPALIVE:-25}" case "${1:-}" in - mask) mask_secrets ;; - up) tunnel_up ;; - down) tunnel_down ;; - *) fail configuration "unknown command '${1:-}'; use mask, up or down." ;; + mask) mask_secrets ;; + up) tunnel_up ;; + down) tunnel_down ;; + *) fail configuration "unknown command '${1:-}'; use mask, up or down." ;; esac } diff --git a/guides/vpn-wireguard.md b/guides/vpn-wireguard.md index 1578f61..ddb02e5 100644 --- a/guides/vpn-wireguard.md +++ b/guides/vpn-wireguard.md @@ -4,7 +4,8 @@ The composite action `actions/connect-wireguard` brings a WireGuard tunnel up on a Gitea Actions runner, so the next steps of the same job (a deployment over SSH, a database migration) can reach hosts that are not on the internet. It is the WireGuard counterpart of the Tailscale action. It is **experimental**: it -lives on a branch until a trial shows it is wanted. +is on `main` but not released, and it has not yet run on a runner with a real +tunnel (MIL-012). A tunnel belongs to one job: every job that deploys brings its own tunnel up and takes it down. It cannot be a reusable workflow, because a called workflow @@ -20,7 +21,7 @@ jobs: - uses: actions/checkout@v4.2.2 - name: Connect the VPN - uses: TirSystem/github-action/actions/connect-wireguard@v2 + uses: TirSystem/github-action/actions/connect-wireguard@main with: config: ${{ secrets.WG_CONFIG }} health-check-host: 10.8.0.1 @@ -30,17 +31,17 @@ jobs: - name: Disconnect the VPN if: always() - uses: TirSystem/github-action/actions/connect-wireguard@v2 + uses: TirSystem/github-action/actions/connect-wireguard@main with: action: down ``` If the short form does not resolve on your instance (Gitea looks for the action on the host set in `DEFAULT_ACTIONS_URL`), use the full address: -`https://git.tirsystem.com/TirSystem/github-action/actions/connect-wireguard@v2`. +`https://git.tirsystem.com/TirSystem/github-action/actions/connect-wireguard@main`. -To try the branch before it is released, replace `@v2` with -`@wireguard-vpn-action`. +Until release 2.2.0 the actions exist only on `main`, so the examples call +`@main`; `@v2` does not contain them yet. After the release, use `@v2`. ## Inputs @@ -71,8 +72,9 @@ To try the branch before it is released, replace `@v2` with A tunnel needs a network interface, so the runner needs more than a plain job container: -- the job runs as root or can use `sudo` (the action installs - `wireguard-tools` with `apt` when it is missing, so a Debian or Ubuntu image); +- the job runs as root, or `sudo` is available (the action uses `sudo` only + when the job is not root, and installs `wireguard-tools` with `apt` when it is + missing, so a Debian or Ubuntu image); - the container has the capability `NET_ADMIN` and the device `/dev/net/tun`. In the `act_runner` configuration set under `container:` `options: "--cap-add=NET_ADMIN --device=/dev/net/tun"`; @@ -85,9 +87,13 @@ privilege. ## What the action does -- masks the keys in the log before anything else; +- masks the keys in the log before anything else (the values of `PrivateKey` + and `PresharedKey` of a full configuration, and the key inputs); +- refuses an input that holds a line break, because an extra line in the file + could add a `PostUp` command that runs as root; - writes `/etc/wireguard/.conf` with mode 600 and brings it up with - `wg-quick`; + `wg-quick` (through `env`, so the `wireguard-go` choice survives `sudo`); the + file is removed again when the interface does not come up; - prints errors as `::error::[category] message`, with the categories `configuration`, `environment` and `connection`; - on `down` takes the interface down and deletes the file. diff --git a/tests/test_connect_wireguard.py b/tests/test_connect_wireguard.py index fc60031..27ea763 100644 --- a/tests/test_connect_wireguard.py +++ b/tests/test_connect_wireguard.py @@ -131,6 +131,40 @@ class TestUp: assert "::error::[connection]" in result.stdout +def make_exec(path: Path, body: str) -> Path: + path.write_text(body, encoding="utf-8") + path.chmod(path.stat().st_mode | stat.S_IXUSR) + return path + + +class TestEnvironment: + def test_the_userspace_variable_reaches_wg_quick_through_sudo(self, tmp_path): + # a sudo that resets the environment, as the real one does + sudo = make_exec( + tmp_path / "sudo-reset", + '#!/usr/bin/env bash\nexec env -i "PATH=$PATH" "STUB_LOG=$STUB_LOG" "$@"\n', + ) + quick = make_exec( + tmp_path / "quick-env", + '#!/usr/bin/env bash\necho "$WG_QUICK_USERSPACE_IMPLEMENTATION $*" >> "$STUB_LOG"\n', + ) + result = run(tmp_path, "up", **FIELDS, WG_SUDO=str(sudo), WG_QUICK=str(quick)) + assert result.returncode == 0, result.stdout + result.stderr + assert "wireguard-go up wg0" in (tmp_path / "log").read_text(encoding="utf-8") + + def test_a_field_with_a_line_break_is_refused(self, tmp_path): + result = run(tmp_path, "up", **{**FIELDS, "WG_ALLOWED_IPS": "10.8.0.0/24\nPostUp = id"}) + assert result.returncode == 1 + assert "WG_ALLOWED_IPS must be one line" in result.stdout + assert not (tmp_path / "etc" / "wg0.conf").exists() + + def test_the_key_file_is_removed_when_wg_quick_fails(self, tmp_path): + failing = make_exec(tmp_path / "quick-fail", "#!/usr/bin/env bash\nexit 1\n") + result = run(tmp_path, "up", **FIELDS, WG_QUICK=str(failing)) + assert result.returncode == 1 + assert not (tmp_path / "etc" / "wg0.conf").exists() + + class TestDownAndMask: def test_down_removes_the_config(self, tmp_path): run(tmp_path, "up", **FIELDS) @@ -141,7 +175,8 @@ class TestDownAndMask: def test_mask_prints_every_secret(self, tmp_path): result = run(tmp_path, "mask", WG_PRIVATE_KEY=KEY, WG_CONFIG="[Interface]\nPrivateKey = abc") assert f"::add-mask::{KEY}" in result.stdout - assert "::add-mask::PrivateKey = abc" in result.stdout + assert "::add-mask::abc" in result.stdout + assert "[Interface]" not in result.stdout # only the key values are masked def test_unknown_command_fails(self, tmp_path): assert run(tmp_path, "sideways").returncode == 1 From 7a30560a7466d77f0ce1716e9598dca8d4944e0f Mon Sep 17 00:00:00 2001 From: Jens Tirsvad Nielsen Date: Mon, 19 Oct 2026 12:01:06 +0800 Subject: [PATCH 2/5] Fix the review findings in connect-tailscale tailscaled is waited for before tailscale up, a tailscaled that already runs on the runner is not taken over and down leaves a daemon it did not start, sudo is used only when the job is not root, an exact version is required to install (with an optional sha256 of the archive), and the guide states when the node is ephemeral and calls @main until the release. Refs #185 Task: MIL-012#4 --- actions/connect-tailscale/action.yml | 9 ++- .../connect-tailscale/connect-tailscale.sh | 74 +++++++++++++++---- guides/vpn-tailscale.md | 42 +++++++---- tests/test_connect_tailscale.py | 51 ++++++++++++- 4 files changed, 142 insertions(+), 34 deletions(-) diff --git a/actions/connect-tailscale/action.yml b/actions/connect-tailscale/action.yml index 9952188..b8510a1 100644 --- a/actions/connect-tailscale/action.yml +++ b/actions/connect-tailscale/action.yml @@ -28,9 +28,13 @@ inputs: required: false default: "" version: - description: "Tailscale version to install when tailscale is not on the runner, e.g. 1.80.2. Pin it; latest is allowed" + description: "Exact Tailscale version to install when tailscale is not on the runner, e.g. 1.80.2. Empty uses the installed tailscale and fails when there is none" required: false - default: "latest" + default: "" + sha256: + description: "Optional SHA-256 of the downloaded archive tailscale__.tgz; the install fails when it differs" + required: false + default: "" userspace-networking: description: >- true runs tailscaled without a TUN device (no NET_ADMIN needed); tailnet @@ -70,6 +74,7 @@ runs: TS_TAGS: ${{ inputs.tags }} TS_HOSTNAME: ${{ inputs.hostname }} TS_VERSION: ${{ inputs.version }} + TS_SHA256: ${{ inputs.sha256 }} TS_USERSPACE: ${{ inputs.userspace-networking }} TS_ARGS: ${{ inputs.args }} TS_TIMEOUT: ${{ inputs.timeout }} diff --git a/actions/connect-tailscale/connect-tailscale.sh b/actions/connect-tailscale/connect-tailscale.sh index f673e84..917f292 100644 --- a/actions/connect-tailscale/connect-tailscale.sh +++ b/actions/connect-tailscale/connect-tailscale.sh @@ -7,14 +7,24 @@ # # Settings come from the environment, never from interpolated expressions: # TS_AUTHKEY, TS_OAUTH_SECRET, TS_TAGS, TS_HOSTNAME, TS_VERSION, TS_USERSPACE, -# TS_ARGS, TS_TIMEOUT, TS_HEALTH_CHECK_HOST. TS_TAILSCALE, TS_TAILSCALED, -# TS_SUDO and TS_STATE_DIR name the commands and the work folder, for the tests. +# TS_SHA256, TS_ARGS, TS_TIMEOUT, TS_HEALTH_CHECK_HOST. TS_TAILSCALE, +# TS_TAILSCALED, TS_SUDO, TS_STATE_DIR, TS_START_WAIT (tries) and +# TS_WAIT_INTERVAL (seconds) name the commands, the work folder and the waits, +# for the tests. # The auth key is passed to tailscale in a file of mode 600, never on a command # line. Every failure is printed as ::error::[category] message. TS_TAILSCALE="${TS_TAILSCALE:-tailscale}" TS_TAILSCALED="${TS_TAILSCALED:-tailscaled}" -SUDO="${TS_SUDO-sudo}" +# sudo is used only when the job is not root: a job container runs as root and +# often has no sudo. TS_SUDO overrides it (empty for none). +if [ -n "${TS_SUDO+x}" ]; then + SUDO="$TS_SUDO" +elif [ "$(id -u)" = "0" ]; then + SUDO="" +else + SUDO="sudo" +fi STATE_DIR="${TS_STATE_DIR:-${RUNNER_TEMP:-/tmp}/tailscale}" fail() { @@ -46,11 +56,13 @@ validate_inputs() { fi [[ "$TS_TIMEOUT" =~ ^[0-9]+$ ]] || fail configuration "timeout must be a number of seconds." - [[ "$TS_VERSION" =~ ^(latest|[0-9]+\.[0-9]+\.[0-9]+)$ ]] || - fail configuration "version must be latest or like 1.80.2." + [[ "$TS_VERSION" =~ ^([0-9]+\.[0-9]+\.[0-9]+)?$ ]] || + fail configuration "version must be an exact version like 1.80.2." + [[ "${TS_SHA256:-}" =~ ^([0-9a-fA-F]{64})?$ ]] || + fail configuration "sha256 must be 64 hexadecimal digits." case "$TS_USERSPACE" in - true | false) ;; - *) fail configuration "userspace-networking must be true or false." ;; + true | false) ;; + *) fail configuration "userspace-networking must be true or false." ;; esac [[ "${TS_HOSTNAME:-}" =~ ^[a-zA-Z0-9-]*$ ]] || fail configuration "hostname may hold letters, digits and hyphens only." @@ -58,14 +70,16 @@ validate_inputs() { machine_arch() { case "$(uname -m)" in - x86_64 | amd64) echo amd64 ;; - aarch64 | arm64) echo arm64 ;; - *) fail environment "no Tailscale build for $(uname -m)." ;; + x86_64 | amd64) echo amd64 ;; + aarch64 | arm64) echo arm64 ;; + *) fail environment "no Tailscale build for $(uname -m)." ;; esac } install_tools() { command -v "$TS_TAILSCALE" >/dev/null 2>&1 && command -v "$TS_TAILSCALED" >/dev/null 2>&1 && return 0 + [ -n "$TS_VERSION" ] || + fail configuration "tailscale is not on the runner; set version (for example 1.80.2) so it can be installed." local arch url tmp arch="$(machine_arch)" url="https://pkgs.tailscale.com/stable/tailscale_${TS_VERSION}_${arch}.tgz" @@ -73,6 +87,10 @@ install_tools() { tmp="$(mktemp -d)" curl -fsSL "$url" -o "$tmp/tailscale.tgz" || fail environment "could not download $url." + if [ -n "${TS_SHA256:-}" ]; then + echo "${TS_SHA256} $tmp/tailscale.tgz" | sha256sum -c --quiet - || + fail environment "the downloaded archive does not match sha256." + fi tar -xzf "$tmp/tailscale.tgz" -C "$tmp" --strip-components=1 || fail environment "could not unpack the Tailscale archive." $SUDO install -m 755 "$tmp/tailscale" "$tmp/tailscaled" /usr/local/bin/ || @@ -80,13 +98,34 @@ install_tools() { rm -rf "$tmp" } +daemon_answers() { + "$TS_TAILSCALE" version --daemon >/dev/null 2>&1 +} + start_daemon() { + # a daemon that already runs belongs to the host; this action would log it out + if daemon_answers; then + fail environment "a tailscaled already runs on this runner; the action starts its own and must not take over the host's. Use it on a job container." + fi local args=(--state=mem:) if [ "$TS_USERSPACE" = "true" ]; then args+=(--tun=userspace-networking --socks5-server=localhost:1055) fi mkdir -p "$STATE_DIR" $SUDO nohup "$TS_TAILSCALED" "${args[@]}" >"$STATE_DIR/tailscaled.log" 2>&1 & + : >"$STATE_DIR/started-by-action" + wait_for_daemon +} + +# wait_for_daemon: tailscaled needs a moment to create its socket +wait_for_daemon() { + local tries="${TS_START_WAIT:-30}" i + for ((i = 0; i < tries; i++)); do + daemon_answers && return 0 + sleep "${TS_WAIT_INTERVAL:-1}" + done + fail connection "tailscaled did not answer in $tries tries; the last lines of its log: $(tail -n 5 "$STATE_DIR/tailscaled.log" 2>/dev/null | tr ' +' ' ')" } # write_key_file: the auth key in a file of mode 600; prints its path @@ -127,6 +166,11 @@ tailnet_up() { } tailnet_down() { + # only a daemon this action started is logged out and stopped + if [ ! -f "$STATE_DIR/started-by-action" ]; then + echo "This action did not start tailscaled on this runner; nothing to do." + return 0 + fi $SUDO "$TS_TAILSCALE" logout 2>/dev/null || echo "The node was not logged in." $SUDO "$TS_TAILSCALE" down 2>/dev/null || true $SUDO pkill -x "$(basename "$TS_TAILSCALED")" 2>/dev/null || true @@ -138,12 +182,12 @@ main() { set -uo pipefail TS_USERSPACE="${TS_USERSPACE:-false}" TS_TIMEOUT="${TS_TIMEOUT:-60}" - TS_VERSION="${TS_VERSION:-latest}" + TS_VERSION="${TS_VERSION:-}" case "${1:-}" in - mask) mask_secrets ;; - up) tailnet_up ;; - down) tailnet_down ;; - *) fail configuration "unknown command '${1:-}'; use mask, up or down." ;; + mask) mask_secrets ;; + up) tailnet_up ;; + down) tailnet_down ;; + *) fail configuration "unknown command '${1:-}'; use mask, up or down." ;; esac } diff --git a/guides/vpn-tailscale.md b/guides/vpn-tailscale.md index dc796af..082c82a 100644 --- a/guides/vpn-tailscale.md +++ b/guides/vpn-tailscale.md @@ -5,8 +5,8 @@ a Gitea Actions runner, so the next steps of the same job (a deployment over SSH, a migration) can reach hosts on the tailnet. It does what the GitHub marketplace action `tailscale/github-action` does, written for Gitea Actions: that action is built for GitHub and has not been verified on Gitea. It is -**experimental**: it lives on the branch `wireguard-vpn-action` until a trial -shows it is wanted. For a plain tunnel without a third-party service, see +**experimental**: it is on `main` but not released, and it has not yet run on a +runner with a real tailnet (MIL-012). For a plain tunnel without a third-party service, see [WireGuard](vpn-wireguard.md). A node belongs to one job: every job that deploys joins on its own and logs out @@ -23,7 +23,7 @@ jobs: - uses: actions/checkout@v4.2.2 - name: Connect Tailscale - uses: TirSystem/github-action/actions/connect-tailscale@v2 + uses: TirSystem/github-action/actions/connect-tailscale@main with: oauth-secret: ${{ secrets.TS_OAUTH_SECRET }} tags: tag:ci @@ -35,16 +35,17 @@ jobs: - name: Disconnect Tailscale if: always() - uses: TirSystem/github-action/actions/connect-tailscale@v2 + uses: TirSystem/github-action/actions/connect-tailscale@main with: action: down ``` If the short form does not resolve on your instance (Gitea looks for the action on the host set in `DEFAULT_ACTIONS_URL`), use the full address: -`https://git.tirsystem.com/TirSystem/github-action/actions/connect-tailscale@v2`. -To try the branch before it is released, replace `@v2` with -`@wireguard-vpn-action`. +`https://git.tirsystem.com/TirSystem/github-action/actions/connect-tailscale@main`. + +Until release 2.2.0 the actions exist only on `main`, so the examples call +`@main`; `@v2` does not contain them yet. After the release, use `@v2`. ## Inputs @@ -55,7 +56,8 @@ To try the branch before it is released, replace `@v2` with | `oauth-secret` | | The secret of an OAuth client with the scope `auth_keys`, as a secret. Needs `tags` | | `tags` | | ACL tags of the node, `tag:ci` or `tag:ci,tag:deploy`. Required with `oauth-secret` | | `hostname` | runner name | Name of the node in the tailnet | -| `version` | `latest` | The version to install when the runner has no `tailscale`. Pin it | +| `version` | | The exact version to install when the runner has no `tailscale`, for example `1.80.2`. Empty uses the installed one and fails when there is none | +| `sha256` | | The SHA-256 of the downloaded archive `tailscale__.tgz`; the install fails when it differs. Recommended | | `userspace-networking` | `false` | `true` runs without a TUN device | | `args` | | Extra flags for `tailscale up`, for example `--accept-routes`. Never a secret | | `timeout` | `60` | Seconds to wait for the node | @@ -74,15 +76,23 @@ To try the branch before it is released, replace `@v2` with start with `GITEA_` or `GITHUB_`. 4. Rotate the secret when a person with access leaves. -The node is ephemeral and its state is kept in memory, so it disappears from the -tailnet when the job ends, even if the last step does not run. +The state of the node is kept in memory. With an OAuth client the key is +ephemeral, so the node disappears from the tailnet when the job ends, even if +the last step does not run. With an auth key that is not ephemeral, a cancelled +job can leave the node in the tailnet until `down` runs or it is removed in the +admin console: create the key as ephemeral. ## The runner -- The job runs as root or can use `sudo`. The action downloads the Tailscale - binaries from `pkgs.tailscale.com` when `tailscale` is not on the runner. - Pin `version`; a runner image that already contains Tailscale skips the - download. +- The job runs as root, or `sudo` is available (the action uses `sudo` only + when the job is not root). The action downloads the Tailscale binaries from + `pkgs.tailscale.com` when `tailscale` is not on the runner; `version` is then + required, and `sha256` makes the install check the archive. A runner image + that already contains Tailscale skips the download. +- The action starts its own `tailscaled`. It refuses to run when a `tailscaled` + already answers on the runner, because that daemon belongs to the host and + `down` would log it out: use the action on job containers, not on a runner + that runs jobs directly on a host with its own Tailscale. - **TUN mode** (the default) needs the capability `NET_ADMIN` and the device `/dev/net/tun` in the runner container: in the `act_runner` configuration set under `container:` `options: "--cap-add=NET_ADMIN --device=/dev/net/tun"`. @@ -107,7 +117,9 @@ tailnet when the job ends, even if the last step does not run. never on a command line, and deletes the file at once; - prints errors as `::error::[category] message`, with the categories `configuration`, `environment` and `connection`; -- on `down` logs the node out, stops `tailscaled` and deletes its work folder. +- waits until `tailscaled` answers before `tailscale up` (up to 30 seconds); +- on `down` logs the node out, stops `tailscaled` and deletes its work folder, + and only when this action started that daemon. The `file:` form of `--auth-key` needs a recent Tailscale; with an old pinned version `tailscale up` reports an invalid key. diff --git a/tests/test_connect_tailscale.py b/tests/test_connect_tailscale.py index 7c9e077..aa64c30 100644 --- a/tests/test_connect_tailscale.py +++ b/tests/test_connect_tailscale.py @@ -36,6 +36,11 @@ case "$1" in done exit "${UP_EXIT:-0}" ;; ping) exit "${PING_EXIT:-0}" ;; + version) + # the daemon answers only after tailscaled was started, and after DAEMON_DELAY calls + [ -f "$STUB_DAEMON" ] || exit 1 + n=$(( $(cat "$STUB_DAEMON") + 1 )); echo "$n" > "$STUB_DAEMON" + [ "$n" -gt "${DAEMON_DELAY:-0}" ] ;; esac """ @@ -52,13 +57,19 @@ def make_stub(path: Path, body: str) -> None: def run(tmp_path: Path, command: str, **env: str) -> subprocess.CompletedProcess: make_stub(tmp_path / "tailscale", TAILSCALE_STUB) - make_stub(tmp_path / "tailscaled", "#!/usr/bin/env bash\necho \"$*\" >> \"$STUB_LOG\"\n") + make_stub( + tmp_path / "tailscaled", + '#!/usr/bin/env bash\necho "$*" >> "$STUB_LOG"\necho 0 > "$STUB_DAEMON"\n', + ) full = { "PATH": f"{tmp_path}:{os.environ['PATH']}", "TS_SUDO": "", "TS_STATE_DIR": str(tmp_path / "state"), "STUB_LOG": str(tmp_path / "log"), "STUB_KEY": str(tmp_path / "key"), + "STUB_DAEMON": str(tmp_path / "daemon"), + "TS_WAIT_INTERVAL": "0.05", + "TS_START_WAIT": "100", **env, } return subprocess.run( @@ -77,7 +88,7 @@ class TestDefinition: def test_inputs_are_the_contract(self, document): assert set(document["inputs"]) == { "action", "authkey", "oauth-secret", "tags", "hostname", "version", - "userspace-networking", "args", "timeout", "health-check-host", + "sha256", "userspace-networking", "args", "timeout", "health-check-host", } def test_uses_gitea_contexts_only(self): @@ -124,6 +135,8 @@ class TestUp: ({"TS_AUTHKEY": AUTHKEY, "TS_TAGS": "ci"}, "tags must look like"), ({"TS_AUTHKEY": AUTHKEY, "TS_TIMEOUT": "soon"}, "timeout"), ({"TS_AUTHKEY": AUTHKEY, "TS_VERSION": "1.x"}, "version"), + ({"TS_AUTHKEY": AUTHKEY, "TS_VERSION": "latest"}, "version"), + ({"TS_AUTHKEY": AUTHKEY, "TS_SHA256": "abc"}, "sha256"), ({"TS_AUTHKEY": AUTHKEY, "TS_USERSPACE": "yes"}, "userspace-networking"), ({"TS_AUTHKEY": AUTHKEY, "TS_HOSTNAME": "bad name"}, "hostname"), ], @@ -148,6 +161,40 @@ class TestUp: assert "health-check-host" in result.stdout +class TestDaemon: + def test_waits_until_the_daemon_answers(self, tmp_path): + result = run(tmp_path, "up", TS_AUTHKEY=AUTHKEY, DAEMON_DELAY="5") + assert result.returncode == 0, result.stdout + assert "up --auth-key" in log(tmp_path) + + def test_a_daemon_that_never_answers_fails_before_up(self, tmp_path): + result = run(tmp_path, "up", TS_AUTHKEY=AUTHKEY, DAEMON_DELAY="1000", TS_START_WAIT="3") + assert result.returncode == 1 + assert "::error::[connection] tailscaled did not answer" in result.stdout + assert "up --auth-key" not in log(tmp_path) + + def test_a_daemon_of_the_host_is_not_taken_over(self, tmp_path): + (tmp_path / "daemon").write_text("0", encoding="utf-8") + result = run(tmp_path, "up", TS_AUTHKEY=AUTHKEY) + assert result.returncode == 1 + assert "::error::[environment] a tailscaled already runs" in result.stdout + calls = log(tmp_path) # only the probe ran: no second daemon, no up + assert "--state=mem:" not in calls + assert "up --auth-key" not in calls + + def test_down_leaves_a_daemon_it_did_not_start(self, tmp_path): + (tmp_path / "daemon").write_text("0", encoding="utf-8") + result = run(tmp_path, "down") + assert result.returncode == 0 + assert "nothing to do" in result.stdout + assert not (tmp_path / "log").exists() + + def test_without_tailscale_a_version_is_required(self, tmp_path): + result = run(tmp_path, "up", TS_AUTHKEY=AUTHKEY, TS_TAILSCALE="no-such-tailscale") + assert result.returncode == 1 + assert "::error::[configuration] tailscale is not on the runner; set version" in result.stdout + + class TestDownAndMask: def test_down_logs_out_and_removes_the_state(self, tmp_path): run(tmp_path, "up", TS_AUTHKEY=AUTHKEY) From a91e0233c1979cf7e8645aa2edb7d2711687cf6c Mon Sep 17 00:00:00 2001 From: Jens Tirsvad Nielsen Date: Mon, 19 Oct 2026 12:01:11 +0800 Subject: [PATCH 3/5] Lint the shell scripts of the actions in the validation run tools/lint-action-scripts.sh runs shellcheck and shfmt on actions/**/*.sh with pinned tools; validate-workflows.yml runs it, and .editorconfig sets the shell format. The tests show a shellcheck problem and a badly formatted script fail. Refs #187 Task: MIL-012#6 --- .editorconfig | 5 ++ .gitea/workflows/validate-workflows.yml | 6 ++ tests/requirements.txt | 2 + tests/test_lint_action_scripts.py | 86 +++++++++++++++++++++++++ tools/lint-action-scripts.sh | 44 +++++++++++++ 5 files changed, 143 insertions(+) create mode 100644 tests/test_lint_action_scripts.py create mode 100644 tools/lint-action-scripts.sh diff --git a/.editorconfig b/.editorconfig index 58de178..d3c791b 100644 --- a/.editorconfig +++ b/.editorconfig @@ -5,3 +5,8 @@ root = true end_of_line = lf charset = utf-8 insert_final_newline = true + +[*.sh] +indent_style = space +indent_size = 2 +switch_case_indent = true diff --git a/.gitea/workflows/validate-workflows.yml b/.gitea/workflows/validate-workflows.yml index e31f372..68defb6 100644 --- a/.gitea/workflows/validate-workflows.yml +++ b/.gitea/workflows/validate-workflows.yml @@ -26,3 +26,9 @@ jobs: - name: Check the coverage of the tools run: .venv/bin/python -m coverage report + + - name: Lint the shell scripts of the actions + env: + SHELLCHECK: .venv/bin/shellcheck + SHFMT: .venv/bin/shfmt + run: bash tools/lint-action-scripts.sh actions diff --git a/tests/requirements.txt b/tests/requirements.txt index e18e2b4..cfb4328 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,3 +1,5 @@ pytest>=8,<10 PyYAML>=6,<7 coverage>=7,<8 +shellcheck-py==0.10.0.1 +shfmt-py==3.11.0.2 diff --git a/tests/test_lint_action_scripts.py b/tests/test_lint_action_scripts.py new file mode 100644 index 0000000..a13f63c --- /dev/null +++ b/tests/test_lint_action_scripts.py @@ -0,0 +1,86 @@ +"""Offline tests for tools/lint-action-scripts.sh (MIL-012 task 6). + +The script runs shellcheck and shfmt on a folder; the tests give it a clean +script, a script with a shellcheck problem and a badly formatted one, so a +seeded error is shown to fail the run. +""" + +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml + +from workflow_source import REPO_ROOT + +SCRIPT = REPO_ROOT / "tools" / "lint-action-scripts.sh" +WORKFLOW = REPO_ROOT / ".gitea" / "workflows" / "validate-workflows.yml" +BASH = shutil.which("bash") +# the tools are in the virtual environment of the interpreter that runs the tests +SEARCH_PATH = os.pathsep.join([str(Path(sys.executable).parent), os.environ.get("PATH", "")]) +SHELLCHECK = shutil.which("shellcheck", path=SEARCH_PATH) +SHFMT = shutil.which("shfmt", path=SEARCH_PATH) +HAVE_TOOLS = BASH and SHELLCHECK and SHFMT + +CLEAN = '#!/usr/bin/env bash\nmain() {\n echo "$1"\n}\n\nmain "$@"\n' +SHELLCHECK_PROBLEM = '#!/usr/bin/env bash\nmain() {\n echo $1\n}\n\nmain "$@"\n' +BADLY_FORMATTED = '#!/usr/bin/env bash\nmain() {\n echo "$1"\n}\n\nmain "$@"\n' + + +def lint(folder: Path) -> subprocess.CompletedProcess: + # shfmt reads .editorconfig from the folder upwards; a temporary folder is + # outside this repository, so it gets a copy of the repository's file + if folder != REPO_ROOT / "actions": + shutil.copy(REPO_ROOT / ".editorconfig", folder / ".editorconfig") + env = {**os.environ, "SHELLCHECK": str(SHELLCHECK), "SHFMT": str(SHFMT)} + return subprocess.run( + [BASH, str(SCRIPT), str(folder)], + capture_output=True, + text=True, + check=False, + cwd=REPO_ROOT, + env=env, + ) + + +@pytest.mark.skipif(not HAVE_TOOLS, reason="bash, shellcheck and shfmt are needed") +class TestLint: + def test_a_clean_script_passes(self, tmp_path): + (tmp_path / "ok.sh").write_bytes(CLEAN.encode()) # LF on every platform + result = lint(tmp_path) + assert result.returncode == 0, result.stdout + result.stderr + assert "clean" in result.stdout + + def test_a_shellcheck_problem_fails(self, tmp_path): + (tmp_path / "bad.sh").write_bytes(SHELLCHECK_PROBLEM.encode()) # LF on every platform + result = lint(tmp_path) + assert result.returncode == 1 + assert "::error::[lint] shellcheck" in result.stdout + + def test_a_badly_formatted_script_fails(self, tmp_path): + (tmp_path / "ugly.sh").write_bytes(BADLY_FORMATTED.encode()) # LF on every platform + result = lint(tmp_path) + assert result.returncode == 1 + assert "::error::[lint] shfmt" in result.stdout + + def test_no_scripts_is_not_a_failure(self, tmp_path): + result = lint(tmp_path) + assert result.returncode == 0 + assert "nothing was checked" in result.stdout + + def test_the_scripts_of_the_actions_are_clean(self): + result = lint(REPO_ROOT / "actions") + assert result.returncode == 0, result.stdout + result.stderr + + +class TestWorkflow: + def test_validate_workflows_runs_the_lint_with_pinned_tools(self): + document = yaml.safe_load(WORKFLOW.read_text(encoding="utf-8")) + commands = [step.get("run", "") for step in document["jobs"]["validate"]["steps"]] + assert any("tools/lint-action-scripts.sh" in command for command in commands) + requirements = (REPO_ROOT / "tests" / "requirements.txt").read_text(encoding="utf-8") + assert "shellcheck-py==" in requirements + assert "shfmt-py==" in requirements diff --git a/tools/lint-action-scripts.sh b/tools/lint-action-scripts.sh new file mode 100644 index 0000000..1e680d5 --- /dev/null +++ b/tools/lint-action-scripts.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# Lint the shell scripts of the composite actions (MIL-012 task 6): shellcheck, +# and shfmt in the format .editorconfig sets (two spaces, indented case labels). +# validate-workflows.yml runs it on every pull request and push to main. +# +# Usage: lint-action-scripts.sh [] (default: actions) +# Env: SHELLCHECK, SHFMT the commands to run (default: from the PATH) +# Exit: 0 every script is clean (or there is none); 1 at least one finding or +# a missing tool +# Needs: bash, find, shellcheck, shfmt + +SHELLCHECK="${SHELLCHECK:-shellcheck}" +SHFMT="${SHFMT:-shfmt}" + +main() { + set -u + local dir="${1:-actions}" failed=0 tool + for tool in "$SHELLCHECK" "$SHFMT"; do + command -v "$tool" >/dev/null 2>&1 || { + echo "::error::[environment] $tool is not installed." + exit 1 + } + done + local files=() + while IFS= read -r file; do + files+=("$file") + done < <(find "$dir" -name '*.sh' -type f | sort) + if [ "${#files[@]}" -eq 0 ]; then + echo "::notice::No shell scripts under '$dir'; nothing was checked." + exit 0 + fi + "$SHELLCHECK" "${files[@]}" || { + echo "::error::[lint] shellcheck found problems." + failed=1 + } + "$SHFMT" -d "${files[@]}" || { + echo "::error::[lint] shfmt would change a script; run shfmt -w." + failed=1 + } + [ "$failed" -eq 0 ] && echo "Checked ${#files[@]} script(s): clean." + exit "$failed" +} + +main "$@" From 3325bea772304b54e8dfffad592b4afadc3ec2b8 Mon Sep 17 00:00:00 2001 From: Jens Tirsvad Nielsen Date: Mon, 19 Oct 2026 12:01:18 +0800 Subject: [PATCH 4/5] Record the MIL-012 results of tasks 1 and 6, ADR-0011 and RC-082 Task 1: both forms of uses: resolve a subfolder action at main. Adds ADR-0011 (Proposed, not yet reviewed), the review record RC-082 for the milestone results, and the traceability rows. Refs #182 Refs #190 --- docs/adr/adr-0011-vpn-composite-actions.md | 67 +++++++++++++++++++ docs/artifact-registry.md | 4 +- .../mil-012-vpn-actions-for-deployment.md | 29 +++++++- docs/sqa/reviews/rc-082-mil-012-results.md | 64 ++++++++++++++++++ docs/sqa/traceability-matrix.md | 11 +-- 5 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 docs/adr/adr-0011-vpn-composite-actions.md create mode 100644 docs/sqa/reviews/rc-082-mil-012-results.md diff --git a/docs/adr/adr-0011-vpn-composite-actions.md b/docs/adr/adr-0011-vpn-composite-actions.md new file mode 100644 index 0000000..8a0670a --- /dev/null +++ b/docs/adr/adr-0011-vpn-composite-actions.md @@ -0,0 +1,67 @@ +# ADR-0011: A VPN for deployment is delivered as a composite action + +## Metadata +| Key | Value | +| --- | --- | +| ID | ADR-0011 | +| CrossReference | [MIL-012], [ADR-0005], [ADR-0006] | + +## Version History +| Date | Status | Author | Reviewer | Change | Commit | +| --- | --- | --- | --- | --- | --- | +| 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Initial version | pending | + +Allowed Status values: `Proposed`, `Accepted`, `Rejected`, `Deprecated`, `Superseded by ADR-NNNN`. + +--- + +## Context + +A deployment job often has to reach hosts that are not on the internet. [MIL-012] adds a way to bring a VPN up in the job, for WireGuard and for Tailscale. The GitHub marketplace has `tailscale/github-action` for Tailscale; it is written for GitHub Actions, has not been verified on Gitea Actions, and WireGuard has no such action. Every other shared automation of this repository is a reusable workflow ([ADR-0005], [ADR-0009], [ADR-0010]), so the question is whether a VPN is one too. + +Options: + +1. **A reusable workflow.** Consistent with the rest of the repository. But a called workflow runs as a separate job, on its own runner and network; a tunnel brought up there is gone when that job ends and is never seen by the caller's deployment job. It cannot work. +2. **The marketplace action `tailscale/github-action`.** No code to maintain. Written for GitHub, not verified on Gitea, nothing for WireGuard, and an action from outside the instance that receives the VPN secret. +3. **A composite action of this repository**, one for each VPN, in `actions//`, called as a step of the caller's own job with `uses:` and `gitea.action_path` for its script. It runs in the job, so the tunnel is there for the steps after it. + +Facts: a composite action in a subfolder of this repository resolves on the instance in both the short form (`TirSystem/github-action/actions/@`) and with the full address, and `gitea.action_path` points at its folder (verified in [MIL-012] task 1, runs 417 and 418 of the instance). A key must not reach a log or a command line. A tunnel needs a network interface, so the runner container needs `NET_ADMIN` and `/dev/net/tun`, which this repository cannot give. + +## Decision + +A VPN for deployment is delivered as **a composite action of this repository**, one per VPN: `actions/connect-wireguard` and `actions/connect-tailscale`. + +- Each action has a script file next to its `action.yml` that does the work. The `run:` lines only call it, and every input reaches the script as an environment variable, never as an interpolated expression ([ADR-0005]). +- Each action has an `up` and a `down` mode. The caller runs `down` in a last step with `if: always()`. +- Secrets are masked first, written only to a file of mode 600 where a file is needed, never printed and never put on a command line. +- The scripts are linted (shellcheck and shfmt) and tested offline with stub commands on every pull request. +- The actions are versioned with the repository ([ADR-0006]); a new action is a minor release, and callers pin the tag. +- The runner is outside the scope of the action: the operator (S03) provides a runner whose container has `NET_ADMIN` and `/dev/net/tun`. The actions state the requirement and fail with a message that names it. + +## Consequences + +**Positive:** + +- The tunnel lives in the job that deploys, which a reusable workflow cannot do. +- One implementation for Gitea, covered by offline tests; no GitHub-only action receives the VPN secret. +- WireGuard and Tailscale share one contract (`up`, `down`, a health check, masked secrets, categorised errors). + +**Negative:** + +- Two scripts to maintain, and behaviour that stubs cannot show (daemon start-up, `sudo`, the runner's capabilities) is verified only by runs on a real runner ([MIL-012] tasks 3 and 4). +- A composite action cannot declare a post step, so the caller must remember the `down` step; a cancelled job can leave a node or a key file behind until the runner cleans up. +- The action needs a privileged runner, which widens the attack surface of that runner; the guides tell the operator to keep it to a dedicated label. + +## Affected Artifacts + +- [MIL-012] — the decision behind its deliverable and its tasks 3 and 4. +- [ADR-0005] — the same contract style (script file, environment variables, categorised failures) is used for the actions. +- [ADR-0006] — a new action is a minor release. + +--- + +[MIL-012]: ../milestones/mil-012-vpn-actions-for-deployment.md +[ADR-0005]: ./adr-0005-quality-check-contract.md +[ADR-0006]: ./adr-0006-release-process-and-version-pinning.md +[ADR-0009]: ./adr-0009-wiki-publishing-reusable-workflow.md +[ADR-0010]: ./adr-0010-code-metrics-badges-contract.md diff --git a/docs/artifact-registry.md b/docs/artifact-registry.md index 94df088..92460e4 100644 --- a/docs/artifact-registry.md +++ b/docs/artifact-registry.md @@ -11,9 +11,9 @@ each document lives in *this* project and the next version to use. | SA | Stakeholder Analysis | docs/stakeholder-analysis.md | 002 | | PP | Project Plan | docs/project-plan.md | 002 | | MIL | Milestone / Gateway | docs/milestones/*.md | 013 | -| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 082 | +| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 083 | | TM | Traceability Matrix | docs/sqa/traceability-matrix.md | 002 | -| ADR | Architecture Decision Record | docs/adr/adr-NNNN-*.md | 0011 | +| ADR | Architecture Decision Record | docs/adr/adr-NNNN-*.md | 0012 | | UCD | Use Case Diagram | docs/use-case-diagram.md | 002 | | US | User Story | docs/user-stories.md | 002 | | UC | Use Case | docs/uc-NNN/uc.md | 009 | diff --git a/docs/milestones/mil-012-vpn-actions-for-deployment.md b/docs/milestones/mil-012-vpn-actions-for-deployment.md index 97ff53b..8bb30ac 100644 --- a/docs/milestones/mil-012-vpn-actions-for-deployment.md +++ b/docs/milestones/mil-012-vpn-actions-for-deployment.md @@ -4,12 +4,13 @@ | Key | Value | | --- | --- | | ID | MIL-012 | -| CrossReference | [BC-001], [US-001], [MIL-004], [MIL-011], [ADR-0006] | +| CrossReference | [BC-001], [US-001], [MIL-004], [MIL-011], [ADR-0006], [ADR-0011] | ## Version History | Date | Status | Author | Reviewer | Change | Commit | | --- | --- | --- | --- | --- | --- | -| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Initial version; reviewed (RC-080), accepted by the author in chat as stand-in for S01 with target date 2026-11-06 | [c5d5941] | +| 2026-10-19 | Deprecated | Jens Tirsvad Nielsen | S01 | Initial version; reviewed (RC-080), accepted by the author in chat as stand-in for S01 with target date 2026-11-06 | [c5d5941] | +| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Added the results of tasks 1, 6 and 9 and the fixes from the review of the first pull request; reviewed (RC-082), accepted by the author in chat as stand-in for S01 | pending | --- @@ -86,6 +87,28 @@ Release `2.2.0` of this repository with two composite actions, `actions/connect- | 11 | Pin the framework templates | In `TirSystem/sqa-qc-framework`, change `@wireguard-vpn-action` to `@v2` in the two templates and the branch guide, remove the "experimental" notes that the release resolves, and open the pull request after release 2.2.0. This is a change in another repository, in its own pull request. | No | | | 12 | Review | Record `RC-*` for this milestone, the actions with their tests, and the guides, each with a verdict. | No | | + +## Verification results + +Task 1 was run on 2026-10-19 with the throwaway branch `verify-subfolder-action`, which calls both actions in the short form (`TirSystem/github-action/actions/@`) and with the full address (`https://git.tirsystem.com/TirSystem/github-action/actions/@`), with every input empty, so that a resolved action fails with its own `[configuration]` message. + +| Call | Result | Evidence | +| --- | --- | --- | +| Either form at `@wireguard-vpn-action` | Fails before the step: `Unable to resolve wireguard-vpn-action: reference not found`. The branch had been deleted when its pull request merged, so the ref did not exist; this is not a defect of the form. | run 417 (`2424`) | +| Short form at `@main`, both actions | Resolves and runs: the log shows `Run Pre ...@main`, the action's `mask` step and its main step, which fails with `[configuration] set config, or all of private-key, ... (WG_PRIVATE_KEY is empty)` and `[configuration] give authkey or oauth-secret` | run 418 (`2425`), jobs `wg-short`, `ts-short` | +| Full address at `@main`, both actions | The same result as the short form | run 418, jobs `wg-full`, `ts-full` | + +Both forms resolve. The short form is the one the guides show; the full address stays in the guides as the fallback. `gitea.action_path` is set (the script path in the log is under `/var/run/act/actions//actions//`). A `uses:` of a branch that no longer exists fails with "reference not found", so a caller of an experimental ref breaks when its branch is deleted. + +Tasks 6 and 9 were done on 2026-10-19 on the branch `mil-012-review-fixes-and-docs`: + +- **Lint (task 6):** `tools/lint-action-scripts.sh` runs shellcheck and shfmt on `actions/**/*.sh`; `validate-workflows.yml` runs it with pinned tools (`shellcheck-py==0.10.0.1`, `shfmt-py==3.11.0.2`, from `tests/requirements.txt`), and `.editorconfig` sets the shell format (two spaces, indented case labels, as the shell conventions say). `tests/test_lint_action_scripts.py` seeds a shellcheck problem and a badly formatted script and shows both fail, and shows the scripts of the actions pass. +- **Decision (task 9):** [ADR-0011] (Proposed). Objective O6 and the Project Plan entries were added at the acceptance of this milestone ([RC-080], [RC-081]). + +The review of pull request 194 (ten findings) is answered in the same branch: `tailscaled` is waited for before `tailscale up`; the `wireguard-go` choice survives `sudo`; `sudo` is used only when the job is not root; a `tailscaled` that already runs on the runner is not taken over and `down` leaves a daemon it did not start; the Tailscale install needs an exact `version`, with an optional `sha256`; a configuration field with a line break is refused; only the key values are masked; the key file is removed when `wg-quick` fails; and the guides call `@main` until the release. These are offline-tested with stubs; tasks 3 and 4 still have to show them on a runner. + +Tasks 2, 3, 4, 5, 7, 8, 10, 11 and 12 are open: they need the trial runner (task 2), which is the operator's, or follow the verified guides (tasks 7 and 8) or the release (tasks 10 and 11). + --- [BC-001]: ../business-case.md @@ -98,6 +121,8 @@ Release `2.2.0` of this repository with two composite actions, `actions/connect- [MIL-012]: ./mil-012-vpn-actions-for-deployment.md [RC-080]: ../sqa/reviews/rc-080-mil-012-plan.md [RC-081]: ../sqa/reviews/rc-081-bc-001-o6.md +[RC-082]: ../sqa/reviews/rc-082-mil-012-results.md [ADR-0006]: ../adr/adr-0006-release-process-and-version-pinning.md +[ADR-0011]: ../adr/adr-0011-vpn-composite-actions.md [PP-001]: ../project-plan.md [c5d5941]: https://git.tirsystem.com/TirSystem/github-action/commit/c5d59418698316866807388bf61dcb6166d9fa8b diff --git a/docs/sqa/reviews/rc-082-mil-012-results.md b/docs/sqa/reviews/rc-082-mil-012-results.md new file mode 100644 index 0000000..470e608 --- /dev/null +++ b/docs/sqa/reviews/rc-082-mil-012-results.md @@ -0,0 +1,64 @@ +# Review Record: MIL-012 results of tasks 1, 6 and the review fixes (MIL-012) + +## Metadata +| Key | Value | +| --- | --- | +| ID | RC-082 | +| CrossReference | [MIL-012], [QC-MIL-001] | + +## Version History +| Date | Status | Author | Reviewer | Change | Commit | +| --- | --- | --- | --- | --- | --- | +| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Initial version; accepted by the author in chat as stand-in for S01 | pending | + +--- + +## Artifact Under Review + +- Instance reviewed: [MIL-012] +- Checklist used: [QC-MIL-001] +- Scope: delta re-review of the milestone after [RC-080]: the new `## Verification results` section (task 1, task 6, the fixes made after the review of pull request 194), the added `[ADR-0011]` in the CrossReference, and the claims about open tasks. Criteria 1 to 6 are re-read; none of the milestone's own text for them changed. [ADR-0011] is **not** reviewed here: it stays `Proposed` and needs a review against its own checklist +- Language and domain: n/a (technical type) +- Mode: supervised, not independent (the author, Jens Tirsvad Nielsen, asked in chat on 2026-10-19 for the acceptance and stands in for S01; the AI prepared the evidence) + +## Checklist Results + +| # | Criterion | Status | Evidence/Notes | +| --- | --- | --- | --- | +| 1 | A concrete deliverable is defined for every gate | Pass | Unchanged: release 2.2.0 with the two actions, guides, wiki pages in three languages, ADR, framework templates pinned. The results section does not claim any of it is delivered; it lists tasks 2 to 5, 7, 8 and 10 to 12 as open. | +| 2 | Explicit Go/No-Go criteria are stated for each gate | Pass | Unchanged, ten criteria. The results bear on two of them. Criterion 1 (a `uses:` of a subfolder action resolves, the form recorded): runs 417 and 418 show both forms resolve at `@main`, and the failure at the deleted branch is explained by the log text "reference not found". Criterion 6 (scripts linted in CI, a seeded error fails): met by `tools/lint-action-scripts.sh` and its tests on the working branch; the pull-request run of the new step is the final evidence. The other eight criteria are not claimed. | +| 3 | Dependencies on other milestones are explicitly mapped | Pass | Unchanged; [ADR-0011] is added to the CrossReference and exists. | +| 4 | Each milestone is traceable to a Business Case objective or KPI | Pass | Unchanged: objective O6 and success criterion 6 of [BC-001], reviewed in [RC-081]. | +| 5 | Milestone owner and approving reviewer are identified | Pass | Unchanged: owner S05, approving reviewer S01. | +| 6 | Milestone has a defined target date consistent with project constraints | Pass | Unchanged: 2026-11-06. The open tasks 2 to 5 need the operator's runner (S03); the milestone states the dependency, and the date is at risk until the runner exists. | + +## Results check + +| # | Claim in the results | Status | Evidence/Notes | +| --- | --- | --- | --- | +| 1 | Task 1: both forms resolve at `@main` | Pass | Run 418 (`.../actions/runs/2425`): jobs `wg-short`, `wg-full`, `ts-short`, `ts-full` each executed the action's `mask` step and main step and failed with the action's own `[configuration]` error. Read from the job logs through the API. | +| 2 | Run 417 failed because the branch was deleted | Pass | `git ls-remote` showed no `wireguard-vpn-action` head after pull request 194 merged; the log says `Unable to resolve wireguard-vpn-action: reference not found`. | +| 3 | Task 6: lint script, workflow step, pinned tools, seeded-error test | Pass | `tools/lint-action-scripts.sh`; the test file seeds a shellcheck problem and a badly formatted script, shows both fail, and shows the two action scripts pass; the full suite (590 tests) passes locally. Not yet run by the instance's CI. | +| 4 | The ten findings of the review of pull request 194 are answered | Pass | Each has a code change and an offline test (the daemon wait with a delayed stub, `sudo` that resets the environment, a host daemon not taken over, a line break refused, the key file removed on failure, `latest` refused). The wording of the results says they are offline-tested with stubs and that tasks 3 and 4 must show them on a runner. | +| 5 | Open tasks are listed honestly | Pass | Tasks 2, 3, 4, 5, 7, 8, 10, 11 and 12 are named as open, with the reason. | + +## Overall Verdict + +Go - the results recorded are accurate and claim no more than the evidence shows. The author confirmed the acceptance in chat on 2026-10-19 as stand-in for S01; this is not an independent review. + +## Action Items + +| Action | Owner | Due | +| --- | --- | --- | +| Review [ADR-0011] against its checklist and accept or change it; it is `Proposed` | S01 | Before task 10 (release 2.2.0) | +| Provide the `vpn` runner, the WireGuard peer and the Tailscale tag with an OAuth client for tasks 3 and 4; the target date 2026-11-06 depends on it | S03 | 2026-10-30 | +| Confirm this record by S01 when an independent reading is possible | S01 | 2026-11-02 | + +--- + +[MIL-012]: ../../milestones/mil-012-vpn-actions-for-deployment.md +[BC-001]: ../../business-case.md +[ADR-0011]: ../../adr/adr-0011-vpn-composite-actions.md +[RC-080]: ./rc-080-mil-012-plan.md +[RC-081]: ./rc-081-bc-001-o6.md +[QC-MIL-001]: ../../../framework/qc/artifacts/qc-milestones-gateways.md diff --git a/docs/sqa/traceability-matrix.md b/docs/sqa/traceability-matrix.md index f2b15ee..735d315 100644 --- a/docs/sqa/traceability-matrix.md +++ b/docs/sqa/traceability-matrix.md @@ -4,13 +4,13 @@ | Key | Value | | --- | --- | | ID | TM-001 | -| CrossReference | [BC-001], [SA-001], [PP-001], [MIL-001], [MIL-002], [MIL-003], [MIL-004], [MIL-005], [MIL-006], [MIL-007], [MIL-008], [MIL-009], [MIL-010], [MIL-011], [MIL-012], [UC-008], [RC-001], [RC-002], [RC-003], [RC-004], [RC-005], [RC-006], [RC-007], [RC-008], [RC-009], [RC-010], [RC-011], [RC-012], [RC-013], [RC-014], [RC-015], [RC-016], [RC-017], [RC-018], [RC-019], [RC-020], [RC-021], [RC-022], [RC-023], [RC-024], [RC-025], [RC-026], [RC-027], [RC-028], [RC-029], [RC-030], [RC-031], [RC-032], [RC-033], [RC-034], [RC-035], [RC-036], [RC-037], [RC-038], [RC-039], [RC-040], [RC-041], [RC-042], [RC-043], [RC-044], [RC-045], [RC-046], [RC-047], [RC-048], [RC-049], [RC-050], [RC-051], [RC-052], [RC-053], [RC-054], [RC-055], [RC-056], [RC-057], [RC-058], [RC-059], [RC-060], [RC-061], [RC-062], [RC-063], [RC-064], [RC-065], [RC-066], [RC-067], [RC-068], [RC-069], [RC-070], [RC-071], [RC-072], [RC-073], [RC-074], [RC-075], [RC-076], [RC-077], [RC-078], [RC-079], [RC-080], [RC-081], [ADR-0007], [ADR-0008], [ADR-0009], [ADR-0010], [UC-007], [UC-006], [ADR-0006], [UC-005], [ADR-0005], [UCD-001], [US-001], [UC-001], [UC-002], [UC-003], [UC-004], [ADR-0001], [ADR-0002], [ADR-0003], [ADR-0004] | +| CrossReference | [BC-001], [SA-001], [PP-001], [MIL-001], [MIL-002], [MIL-003], [MIL-004], [MIL-005], [MIL-006], [MIL-007], [MIL-008], [MIL-009], [MIL-010], [MIL-011], [MIL-012], [ADR-0011], [UC-008], [RC-001], [RC-002], [RC-003], [RC-004], [RC-005], [RC-006], [RC-007], [RC-008], [RC-009], [RC-010], [RC-011], [RC-012], [RC-013], [RC-014], [RC-015], [RC-016], [RC-017], [RC-018], [RC-019], [RC-020], [RC-021], [RC-022], [RC-023], [RC-024], [RC-025], [RC-026], [RC-027], [RC-028], [RC-029], [RC-030], [RC-031], [RC-032], [RC-033], [RC-034], [RC-035], [RC-036], [RC-037], [RC-038], [RC-039], [RC-040], [RC-041], [RC-042], [RC-043], [RC-044], [RC-045], [RC-046], [RC-047], [RC-048], [RC-049], [RC-050], [RC-051], [RC-052], [RC-053], [RC-054], [RC-055], [RC-056], [RC-057], [RC-058], [RC-059], [RC-060], [RC-061], [RC-062], [RC-063], [RC-064], [RC-065], [RC-066], [RC-067], [RC-068], [RC-069], [RC-070], [RC-071], [RC-072], [RC-073], [RC-074], [RC-075], [RC-076], [RC-077], [RC-078], [RC-079], [RC-080], [RC-081], [RC-082], [ADR-0007], [ADR-0008], [ADR-0009], [ADR-0010], [UC-007], [UC-006], [ADR-0006], [UC-005], [ADR-0005], [UCD-001], [US-001], [UC-001], [UC-002], [UC-003], [UC-004], [ADR-0001], [ADR-0002], [ADR-0003], [ADR-0004] | ## Version History | Date | Status | Author | Reviewer | Change | Commit | | --- | --- | --- | --- | --- | --- | -| 2026-10-14 | Proposed | Jens Tirsvad Nielsen | S01 | Added RC-079 (MIL-011 outcome, release 2.1.1) | [4aa3fd3] | | 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Added MIL-012 (VPN actions for deployment), RC-080 and RC-081 (its plan and objective O6); CrossReference lists RC-076 to RC-079 | [c5d5941] | +| 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Added ADR-0011 and RC-082 (MIL-012 results of tasks 1, 6 and the review fixes) | pending | --- @@ -32,6 +32,7 @@ updated whenever an artifact instance is created or reviewed. | [MIL-003] | MIL | [BC-001], [PP-001], [MIL-002] | [MIL-004] | [RC-006], [RC-028], [RC-033], [RC-039] | | [MIL-004] | MIL | [BC-001], [PP-001], [MIL-003] | - | [RC-007], [RC-029], [RC-034], [RC-048] | | [ADR-0010] | ADR | [MIL-010], [UC-008], [US-001], [ADR-0005], [ADR-0006] | - | [RC-072], [RC-073] | +| [ADR-0011] | ADR | [MIL-012], [ADR-0005], [ADR-0006] | - | - | | [RC-001] | RC | [BC-001] | - | - | | [RC-002] | RC | [PP-001] | - | - | | [RC-003] | RC | [MIL-001] | - | - | @@ -45,7 +46,7 @@ updated whenever an artifact instance is created or reviewed. | [MIL-009] | MIL | [BC-001], [PP-001], [MIL-008], [ADR-0004], [ADR-0006], [ADR-0009] | - | [RC-064], [RC-066] | | [MIL-010] | MIL | [BC-001], [PP-001], [MIL-003], [MIL-004] | [MIL-011] | [RC-068], [RC-074], [RC-075] | | [MIL-011] | MIL | [BC-001], [PP-001], [MIL-010], [ADR-0006], [ADR-0010] | [MIL-012] | [RC-076], [RC-077], [RC-078], [RC-079] | -| [MIL-012] | MIL | [BC-001], [PP-001], [MIL-004], [MIL-011], [ADR-0006] | - | [RC-080] | +| [MIL-012] | MIL | [BC-001], [PP-001], [MIL-004], [MIL-011], [ADR-0006], [ADR-0011] | - | [RC-080], [RC-082] | | [RC-007] | RC | [MIL-004] | - | - | | [RC-008] | RC | [BC-001] | - | - | | [RC-009] | RC | [PP-001] | - | - | @@ -140,6 +141,7 @@ updated whenever an artifact instance is created or reviewed. | [RC-079] | RC | [MIL-011] | - | - | | [RC-080] | RC | [MIL-012], [PP-001] | - | - | | [RC-081] | RC | [BC-001] | - | - | +| [RC-082] | RC | [MIL-012] | - | - | ## Coverage Notes @@ -166,6 +168,7 @@ updated whenever an artifact instance is created or reviewed. [MIL-012]: ../milestones/mil-012-vpn-actions-for-deployment.md [MIL-009]: ../milestones/mil-009-migrate-wiki-callers-and-remove-scoped-workflow.md [ADR-0010]: ../adr/adr-0010-code-metrics-badges-contract.md +[ADR-0011]: ../adr/adr-0011-vpn-composite-actions.md [RC-001]: ./reviews/rc-001-business-case.md [RC-002]: ./reviews/rc-002-project-plan.md [RC-003]: ./reviews/rc-003-mil-001.md @@ -257,6 +260,7 @@ updated whenever an artifact instance is created or reviewed. [RC-079]: ./reviews/rc-079-mil-011-outcome.md [RC-080]: ./reviews/rc-080-mil-012-plan.md [RC-081]: ./reviews/rc-081-bc-001-o6.md +[RC-082]: ./reviews/rc-082-mil-012-results.md [ADR-0008]: ../adr/adr-0008-cpp-csharp-quality-check-contract.md [ADR-0009]: ../adr/adr-0009-wiki-publishing-reusable-workflow.md [ADR-0007]: ../adr/adr-0007-release-publish-destination.md @@ -267,5 +271,4 @@ updated whenever an artifact instance is created or reviewed. [UC-005]: ../uc-005/uc.md [ADR-0005]: ../adr/adr-0005-quality-check-contract.md [3d7a9df]: https://git.tirsystem.com/TirSystem/github-action/commit/3d7a9df3a3a4712dcc5214a71b0569d90122bf4e -[4aa3fd3]: https://git.tirsystem.com/TirSystem/github-action/commit/4aa3fd38f97c26a5d52abc566e958759583ccb85 [c5d5941]: https://git.tirsystem.com/TirSystem/github-action/commit/c5d59418698316866807388bf61dcb6166d9fa8b From b75ed03d3f8fe53451446e5f463cc334caad25f5 Mon Sep 17 00:00:00 2001 From: Jens Tirsvad Nielsen Date: Mon, 19 Oct 2026 12:01:24 +0800 Subject: [PATCH 5/5] Resolve pending commit links for MIL-012 results --- docs/adr/adr-0011-vpn-composite-actions.md | 3 ++- docs/milestones/mil-012-vpn-actions-for-deployment.md | 3 ++- docs/sqa/reviews/rc-082-mil-012-results.md | 3 ++- docs/sqa/traceability-matrix.md | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/adr/adr-0011-vpn-composite-actions.md b/docs/adr/adr-0011-vpn-composite-actions.md index 8a0670a..ae7f351 100644 --- a/docs/adr/adr-0011-vpn-composite-actions.md +++ b/docs/adr/adr-0011-vpn-composite-actions.md @@ -9,7 +9,7 @@ ## Version History | Date | Status | Author | Reviewer | Change | Commit | | --- | --- | --- | --- | --- | --- | -| 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Initial version | pending | +| 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Initial version | [3325bea] | Allowed Status values: `Proposed`, `Accepted`, `Rejected`, `Deprecated`, `Superseded by ADR-NNNN`. @@ -65,3 +65,4 @@ A VPN for deployment is delivered as **a composite action of this repository**, [ADR-0006]: ./adr-0006-release-process-and-version-pinning.md [ADR-0009]: ./adr-0009-wiki-publishing-reusable-workflow.md [ADR-0010]: ./adr-0010-code-metrics-badges-contract.md +[3325bea]: https://git.tirsystem.com/TirSystem/github-action/commit/3325bea772304b54e8dfffad592b4afadc3ec2b8 diff --git a/docs/milestones/mil-012-vpn-actions-for-deployment.md b/docs/milestones/mil-012-vpn-actions-for-deployment.md index 8bb30ac..a82c913 100644 --- a/docs/milestones/mil-012-vpn-actions-for-deployment.md +++ b/docs/milestones/mil-012-vpn-actions-for-deployment.md @@ -10,7 +10,7 @@ | Date | Status | Author | Reviewer | Change | Commit | | --- | --- | --- | --- | --- | --- | | 2026-10-19 | Deprecated | Jens Tirsvad Nielsen | S01 | Initial version; reviewed (RC-080), accepted by the author in chat as stand-in for S01 with target date 2026-11-06 | [c5d5941] | -| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Added the results of tasks 1, 6 and 9 and the fixes from the review of the first pull request; reviewed (RC-082), accepted by the author in chat as stand-in for S01 | pending | +| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Added the results of tasks 1, 6 and 9 and the fixes from the review of the first pull request; reviewed (RC-082), accepted by the author in chat as stand-in for S01 | [3325bea] | --- @@ -126,3 +126,4 @@ Tasks 2, 3, 4, 5, 7, 8, 10, 11 and 12 are open: they need the trial runner (task [ADR-0011]: ../adr/adr-0011-vpn-composite-actions.md [PP-001]: ../project-plan.md [c5d5941]: https://git.tirsystem.com/TirSystem/github-action/commit/c5d59418698316866807388bf61dcb6166d9fa8b +[3325bea]: https://git.tirsystem.com/TirSystem/github-action/commit/3325bea772304b54e8dfffad592b4afadc3ec2b8 diff --git a/docs/sqa/reviews/rc-082-mil-012-results.md b/docs/sqa/reviews/rc-082-mil-012-results.md index 470e608..b0ae4f9 100644 --- a/docs/sqa/reviews/rc-082-mil-012-results.md +++ b/docs/sqa/reviews/rc-082-mil-012-results.md @@ -9,7 +9,7 @@ ## Version History | Date | Status | Author | Reviewer | Change | Commit | | --- | --- | --- | --- | --- | --- | -| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Initial version; accepted by the author in chat as stand-in for S01 | pending | +| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Initial version; accepted by the author in chat as stand-in for S01 | [3325bea] | --- @@ -62,3 +62,4 @@ Go - the results recorded are accurate and claim no more than the evidence shows [RC-080]: ./rc-080-mil-012-plan.md [RC-081]: ./rc-081-bc-001-o6.md [QC-MIL-001]: ../../../framework/qc/artifacts/qc-milestones-gateways.md +[3325bea]: https://git.tirsystem.com/TirSystem/github-action/commit/3325bea772304b54e8dfffad592b4afadc3ec2b8 diff --git a/docs/sqa/traceability-matrix.md b/docs/sqa/traceability-matrix.md index 735d315..5159619 100644 --- a/docs/sqa/traceability-matrix.md +++ b/docs/sqa/traceability-matrix.md @@ -10,7 +10,7 @@ | Date | Status | Author | Reviewer | Change | Commit | | --- | --- | --- | --- | --- | --- | | 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Added MIL-012 (VPN actions for deployment), RC-080 and RC-081 (its plan and objective O6); CrossReference lists RC-076 to RC-079 | [c5d5941] | -| 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Added ADR-0011 and RC-082 (MIL-012 results of tasks 1, 6 and the review fixes) | pending | +| 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Added ADR-0011 and RC-082 (MIL-012 results of tasks 1, 6 and the review fixes) | [3325bea] | --- @@ -272,3 +272,4 @@ updated whenever an artifact instance is created or reviewed. [ADR-0005]: ../adr/adr-0005-quality-check-contract.md [3d7a9df]: https://git.tirsystem.com/TirSystem/github-action/commit/3d7a9df3a3a4712dcc5214a71b0569d90122bf4e [c5d5941]: https://git.tirsystem.com/TirSystem/github-action/commit/c5d59418698316866807388bf61dcb6166d9fa8b +[3325bea]: https://git.tirsystem.com/TirSystem/github-action/commit/3325bea772304b54e8dfffad592b4afadc3ec2b8