diff --git a/SECURITY.md b/SECURITY.md index 0736aa8..4a86a33 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -42,3 +42,15 @@ Things we care about most: - Tunnel credentials are stored at `~/.zt/tunnels//.json` with mode `0600` - cfzt never transmits credentials anywhere except the Cloudflare API over HTTPS - The Gitleaks CI scan checks all commits for accidentally leaked secrets +- **`zt down --remote` deletes by name, not by ownership.** Cloudflare + Tunnels have no "created by zt" marker the way a zt-managed DNS + record does, so `--remote` resolves and tears down *whatever tunnel + exists on the account under that exact name* — including one you + created manually via the Cloudflare dashboard, if it happens to + share the name. It's opt-in and off by default (a plain `zt down` + only ever acts on tunnels in local state) specifically so this can't + happen from a typo on your own machine. Reserve `--remote` for + automated contexts — CI tearing down its own PR preview — where the + caller already knows the name is exclusively theirs, and treat + reusing tunnel names between manually-managed and zt-managed tunnels + as unsafe. diff --git a/action.yml b/action.yml index f8a9759..e58620b 100644 --- a/action.yml +++ b/action.yml @@ -55,6 +55,23 @@ inputs: description: 'cloudflared protocol: auto, quic, or http2. mode=up only.' required: false default: 'auto' + cloudflared-version: + description: >- + cloudflared release tag to install (e.g. '2026.7.0'), skipping + the version check on PATH. Leave empty to always install + whatever the "latest" GitHub release currently is — simplest, + but the exact binary a given commit runs against can then change + between one workflow run and the next. Pin this for reproducible + CI. + required: false + default: '' + cfzt-version: + description: >- + cfzt (zt) release tag to install (e.g. 'v0.9.0'). Leave empty to + install main's install.sh at whatever it currently resolves to + "latest" as — same reproducibility caveat as cloudflared-version. + required: false + default: '' force: description: >- Replace a pre-existing DNS record for the hostname that a previous @@ -91,6 +108,7 @@ runs: MODE: ${{ inputs.mode }} DOCKER: ${{ inputs.docker }} PORT: ${{ inputs.port }} + CREATE_DEPLOYMENT: ${{ inputs.create-deployment }} run: | set -euo pipefail case "$MODE" in @@ -101,6 +119,22 @@ runs: echo "::error::port is required for mode=up unless docker is true" exit 1 fi + # jq is used unconditionally (config write, up/down argument + # building); gh only when the Deployments API steps actually run. + # Failing fast here with a clear message beats the composite + # action dying several steps in with a bare "command not found" + # on a self-hosted runner that doesn't have these preinstalled — + # both ship by default on GitHub-hosted Ubuntu runners. + command -v jq >/dev/null 2>&1 || { + echo "::error::jq is required (not found on PATH) — install it or use a runner image that includes it" + exit 1 + } + if [ "$CREATE_DEPLOYMENT" = "true" ]; then + command -v gh >/dev/null 2>&1 || { + echo "::error::gh (GitHub CLI) is required for create-deployment (not found on PATH) — install it, or set create-deployment: 'false' to skip the Deployments UI integration" + exit 1 + } + fi - name: Compute hostname id: hostname @@ -129,6 +163,8 @@ runs: - name: Install cloudflared shell: bash + env: + CLOUDFLARED_VERSION: ${{ inputs.cloudflared-version }} run: | set -euo pipefail if command -v cloudflared >/dev/null 2>&1; then @@ -141,21 +177,40 @@ runs: aarch64|arm64) asset=cloudflared-linux-arm64 ;; *) echo "::error::unsupported architecture for cloudflared: $arch"; exit 1 ;; esac - curl -fsSL "https://github.com/cloudflare/cloudflared/releases/latest/download/${asset}" -o /usr/local/bin/cloudflared + if [ -n "$CLOUDFLARED_VERSION" ]; then + url="https://github.com/cloudflare/cloudflared/releases/download/${CLOUDFLARED_VERSION}/${asset}" + else + echo "::warning::cloudflared-version not set — installing whatever 'latest' currently is. Pin cloudflared-version for reproducible runs." + url="https://github.com/cloudflare/cloudflared/releases/latest/download/${asset}" + fi + curl -fsSL "$url" -o /usr/local/bin/cloudflared chmod +x /usr/local/bin/cloudflared cloudflared --version - name: Install cfzt shell: bash + env: + CFZT_VERSION: ${{ inputs.cfzt-version }} run: | set -euo pipefail if command -v zt >/dev/null 2>&1; then echo "zt already on PATH: $(zt version)" exit 0 fi - curl -fsSL https://raw.githubusercontent.com/casablanque-code/cfzt/main/install.sh | bash + if [ -n "$CFZT_VERSION" ]; then + # Pin install.sh itself to the same tag, not just the binary it + # installs — main's install.sh can change independently of any + # released zt version, so fetching it from main would still + # leave the install *process* unpinned even with ZT_VERSION set. + curl -fsSL "https://raw.githubusercontent.com/casablanque-code/cfzt/${CFZT_VERSION}/install.sh" \ + | ZT_VERSION="$CFZT_VERSION" bash + else + echo "::warning::cfzt-version not set — installing whatever main's install.sh currently resolves 'latest' to. Pin cfzt-version for reproducible runs." + curl -fsSL https://raw.githubusercontent.com/casablanque-code/cfzt/main/install.sh | bash + fi - name: Write cfzt config + id: write-config shell: bash env: CF_API_TOKEN: ${{ inputs.cloudflare-api-token }} @@ -163,6 +218,11 @@ runs: DOMAIN: ${{ inputs.domain }} run: | set -euo pipefail + if [ -e "${HOME}/.zt-config.json" ]; then + echo "created=false" >> "$GITHUB_OUTPUT" + else + echo "created=true" >> "$GITHUB_OUTPUT" + fi jq -n \ --arg api_token "$CF_API_TOKEN" \ --arg account_id "$CF_ACCOUNT_ID" \ @@ -172,6 +232,7 @@ runs: chmod 600 "${HOME}/.zt-config.json" - name: zt up + id: zt-up if: inputs.mode == 'up' shell: bash env: @@ -202,12 +263,14 @@ runs: zt "${args[@]}" - name: Record GitHub deployment + id: record-deployment if: inputs.mode == 'up' && inputs.create-deployment == 'true' shell: bash env: GH_TOKEN: ${{ inputs.github-token }} REPO: ${{ github.repository }} REF: ${{ github.sha }} + NAME: ${{ inputs.name }} ENVIRONMENT: ${{ steps.env-name.outputs.environment }} URL: ${{ steps.hostname.outputs.url }} run: | @@ -216,11 +279,22 @@ runs: # API defers to the repo's required status checks and can refuse # to create the deployment before they've all passed, which isn't # what we want for a preview tunnel that IS one of the checks. + # task is set to "cfzt:" (rather than left as the default + # "deploy") so the "down" step can identify exactly which + # deployments belong to this preview, independent of environment + # or sha. environment alone isn't enough: it can be reused across + # multiple previews (e.g. several PRs all passed `environment: + # preview`), and matching by sha doesn't work either since down + # runs on pull_request:closed with a different sha than whichever + # up call(s) preceded it. name is the one value the action + # requires to stay identical between up and down, so it's the + # only safe join key. payload=$(jq -n \ --arg ref "$REF" \ --arg environment "$ENVIRONMENT" \ --arg description "cfzt preview: $URL" \ - '{ref: $ref, environment: $environment, description: $description, auto_merge: false, required_contexts: []}') + --arg task "cfzt:${NAME}" \ + '{ref: $ref, task: $task, environment: $environment, description: $description, auto_merge: false, required_contexts: []}') deployment_id=$(gh api "repos/${REPO}/deployments" --method POST --input - <<< "$payload" --jq '.id') status_payload=$(jq -n \ @@ -229,6 +303,29 @@ runs: '{state: "success", environment: $environment, environment_url: $environment_url, description: "tunnel live"}') gh api "repos/${REPO}/deployments/${deployment_id}/statuses" --method POST --input - <<< "$status_payload" + - name: Clean up tunnel on deployment-recording failure + # zt up creating a live tunnel while the GitHub Deployment never + # gets recorded leaves Cloudflare state that nothing in the + # workflow knows to tear down later — the corresponding "down" run + # (keyed off task=cfzt:) will never find a deployment to + # react to, and there's no other trigger for it. If the up half + # succeeded but recording it failed, undo the tunnel immediately + # rather than leaving an orphaned preview live indefinitely. + # always() is required here: the default step condition skips this + # step entirely once any prior step has failed, which would also + # skip this cleanup — always() plus the explicit outcome checks + # narrow it back down to exactly the up-succeeded/record-failed case. + if: >- + always() && inputs.mode == 'up' && inputs.create-deployment == 'true' && + steps.zt-up.outcome == 'success' && steps.record-deployment.outcome == 'failure' + shell: bash + env: + NAME: ${{ inputs.name }} + run: | + set -euo pipefail + echo "::warning::GitHub deployment recording failed after zt up succeeded — tearing the tunnel back down" + zt down "$NAME" --remote + - name: zt down if: inputs.mode == 'down' shell: bash @@ -242,13 +339,37 @@ runs: env: GH_TOKEN: ${{ inputs.github-token }} REPO: ${{ github.repository }} + NAME: ${{ inputs.name }} ENVIRONMENT: ${{ steps.env-name.outputs.environment }} run: | set -euo pipefail - ids=$(gh api "repos/${REPO}/deployments?environment=${ENVIRONMENT}" --jq '.[].id') + # Filter by task, not just environment: environment can be reused + # across multiple unrelated previews (custom `environment:` input + # set to the same value for several PRs), and the environment + # query param alone would then mark every one of them inactive. + # task="cfzt:" was set at creation time in the "up" step and + # is unique per preview, so re-check it client-side after the + # environment-scoped list call. + task="cfzt:${NAME}" + ids=$(gh api "repos/${REPO}/deployments?environment=${ENVIRONMENT}" \ + --jq --arg task "$task" '.[] | select(.task == $task) | .id') for id in $ids; do gh api "repos/${REPO}/deployments/${id}/statuses" \ --method POST \ -f "state=inactive" \ -f "environment=${ENVIRONMENT}" done + + - name: Remove cfzt config + # Only removes the file this run itself created (write-config's + # created=true) — never a config that already existed on the + # runner before this action touched it, e.g. a self-hosted runner + # with its own persistent ~/.zt-config.json. always() so this + # still runs after an earlier step failure; on GitHub-hosted + # ephemeral runners the API token dying with the VM makes this + # mostly redundant, but on self-hosted/persistent runners a + # Cloudflare API token left sitting in a world-readable-by-owner + # file after the job ends is unnecessary exposure. + if: always() && steps.write-config.outputs.created == 'true' + shell: bash + run: rm -f "${HOME}/.zt-config.json" diff --git a/install.sh b/install.sh index f4ed46c..723ae7e 100644 --- a/install.sh +++ b/install.sh @@ -14,9 +14,17 @@ case "$ARCH" in *) echo "Unsupported arch: $ARCH"; exit 1 ;; esac -# Resolve latest release tag -TAG=$(curl -fsSL "https://api.github.com/repos/${REPO}/releases/latest" \ - | grep '"tag_name"' | sed 's/.*"tag_name": "\(.*\)".*/\1/') +# Resolve release tag. ZT_VERSION lets a caller (e.g. the cfzt-action +# GitHub Action, or anyone scripting this for CI) pin to a specific +# release instead of always tracking main's idea of "latest" — without +# it, the same `curl install.sh | bash` line can silently install a +# different zt build tomorrow than it did today. +if [[ -n "${ZT_VERSION:-}" ]]; then + TAG="$ZT_VERSION" +else + TAG=$(curl -fsSL "https://api.github.com/repos/${REPO}/releases/latest" \ + | grep '"tag_name"' | sed 's/.*"tag_name": "\(.*\)".*/\1/') +fi if [[ -z "$TAG" ]]; then echo "error: could not resolve latest release tag" >&2 diff --git a/internal/docker/docker.go b/internal/docker/docker.go index 984941a..a778dc1 100644 --- a/internal/docker/docker.go +++ b/internal/docker/docker.go @@ -4,8 +4,11 @@ import ( "encoding/json" "fmt" "io" + "math" "net" "net/http" + "sort" + "strconv" "strings" "time" ) @@ -134,12 +137,50 @@ func findByList(name string) (string, error) { } func pickPort(name string, ports map[string][]portBinding) (string, error) { - for _, bindings := range ports { - for _, b := range bindings { - if b.HostPort != "" { - return b.HostPort, nil - } + // map iteration order is randomized in Go, so picking the first key + // seen made the chosen port nondeterministic across runs whenever a + // container published more than one (e.g. -p 8080:80 -p 8443:443) — + // same container, same `zt up --docker` invocation, different port + // depending on run. Sort keys ("80/tcp", "443/tcp", ...) and take the + // lowest container port among tcp bindings, so the choice is stable + // and matches what a reader would expect ("the app's main port"). + var tcpKeys []string + for k, bindings := range ports { + if len(bindings) == 0 || bindings[0].HostPort == "" { + continue + } + if !strings.HasSuffix(k, "/tcp") { + continue + } + tcpKeys = append(tcpKeys, k) + } + if len(tcpKeys) == 0 { + return "", fmt.Errorf("container %q has no published TCP ports\n hint: start it with -p :", name) + } + sort.Slice(tcpKeys, func(i, j int) bool { + return containerPortNum(tcpKeys[i]) < containerPortNum(tcpKeys[j]) + }) + best := tcpKeys[0] + for _, b := range ports[best] { + if b.HostPort != "" { + return b.HostPort, nil } } return "", fmt.Errorf("container %q has no published ports\n hint: start it with -p :", name) } + +// containerPortNum extracts the numeric container port from a Docker +// ports-map key like "80/tcp" ("443/tcp" -> 443). Unparseable keys sort +// last (return maxInt) rather than erroring — pickPort's job is to make a +// reasonable deterministic choice, not to validate Docker's own output. +func containerPortNum(key string) int { + numPart, _, found := strings.Cut(key, "/") + if !found { + return math.MaxInt + } + n, err := strconv.Atoi(numPart) + if err != nil { + return math.MaxInt + } + return n +} diff --git a/internal/docker/docker_test.go b/internal/docker/docker_test.go index 7bc4740..14763ae 100644 --- a/internal/docker/docker_test.go +++ b/internal/docker/docker_test.go @@ -152,3 +152,31 @@ func TestFindContainerPort_DockerUnreachable(t *testing.T) { t.Fatal("FindContainerPort() = nil error, want error when docker is unreachable") } } + +// Multiple published TCP ports must resolve to the same port every time +// regardless of Go's randomized map iteration order — a container with +// e.g. -p 8080:80 -p 8443:443 should always pick the lowest container +// port (80/tcp here), not whichever key the map happened to yield first. +func TestFindContainerPort_MultiplePorts_Deterministic(t *testing.T) { + withDockerServer(t, func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/version") { + jsonHandler(200, `{"ApiVersion":"1.44"}`)(w, r) + return + } + jsonHandler(200, `{"State":{"Running":true},"NetworkSettings":{"Ports":{ + "443/tcp":[{"HostPort":"8443"}], + "80/tcp":[{"HostPort":"8080"}], + "9000/tcp":[{"HostPort":"9999"}] + }}}`)(w, r) + }) + + for i := 0; i < 20; i++ { + port, err := FindContainerPort("multi-port") + if err != nil { + t.Fatalf("FindContainerPort() error = %v", err) + } + if port != "8080" { + t.Fatalf("FindContainerPort() = %q, want 8080 (lowest container port 80/tcp) on iteration %d", port, i) + } + } +} diff --git a/internal/service/service_windows.go b/internal/service/service_windows.go index 71912b3..dce4b16 100644 --- a/internal/service/service_windows.go +++ b/internal/service/service_windows.go @@ -188,6 +188,26 @@ func waitForTaskRunning(tn string) bool { return false } +// waitForTaskStopped polls tn's state for up to taskStartupBackoff's total +// span and reports whether it left Running. schtasks /end only requests +// termination — it doesn't block until the process is actually gone — so +// without this, Uninstall's subsequent /delete could remove the task +// registration while the cloudflared process it was tracking is still +// alive and holding its port, leaving an orphan indistinguishable from +// any other running process once Task Scheduler stops tracking it. +func waitForTaskStopped(tn string) bool { + if !taskIsRunning(tn) { + return true + } + for _, d := range taskStartupBackoff { + time.Sleep(d) + if !taskIsRunning(tn) { + return true + } + } + return false +} + // tailLog returns the last n lines of the file at path, indented for // display, or a placeholder if it can't be read. Best-effort diagnostic // context for a task that died immediately after starting — not worth @@ -322,6 +342,9 @@ func Uninstall(name string) error { return nil } _, _ = runSchtasks("/end", "/tn", tn) + if !waitForTaskStopped(tn) { + fmt.Printf(" ! %s did not report stopped after /end — deleting its task registration anyway; the cloudflared process may still be running and holding its port (check with `zt doctor` or Task Manager)\n", tn) + } out, err := runSchtasks("/delete", "/tn", tn, "/f") if err != nil { return fmt.Errorf("schtasks /delete: %w\n%s\n%s", err, out, accessDeniedHint(tn)) @@ -386,6 +409,9 @@ func UninstallWatchdog() error { return nil } _, _ = runSchtasks("/end", "/tn", watchdogTaskName) + if !waitForTaskStopped(watchdogTaskName) { + fmt.Printf(" ! %s did not report stopped after /end — deleting its task registration anyway; the watchdog process may still be running (check Task Manager)\n", watchdogTaskName) + } out, err := runSchtasks("/delete", "/tn", watchdogTaskName, "/f") if err != nil { return fmt.Errorf("schtasks /delete: %w\n%s\n%s", err, out, accessDeniedHint(watchdogTaskName))