From 81feffd1e198358488a0c8ea626e8b5fd9321c64 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Fri, 30 Oct 2026 00:00:32 -0700 Subject: [PATCH 1/6] ci(deploy): add railway_deploy.sh so CLI deploys carry commit identity The Railway service has no GitHub source and deploy.yml is disabled, so production is deployed by agents running `railway up` from a checkout. Those uploads carry no RAILWAY_GIT_COMMIT_SHA and nobody stamps build-sha.txt, so /health answers `build.commit_sha: null` and nothing on production says which code is serving. scripts/railway_deploy.sh refuses a dirty tree or a HEAD that is not on origin/main (unless --allow-unmerged), writes the full HEAD SHA into build-sha.txt for the upload only and restores the tracked placeholder in an EXIT trap, runs `railway up --ci` with an explicit service and environment (env vars or `railway status`) and a " @" message, then polls /health until .build.commit_sha equals the shipped SHA. The poll loop and its three failure messages mirror the disabled deploy.yml step, with the 900 s budget and 15 s interval exposed as env vars so the test can exercise the timeout path. tests/scripts/test_railway_deploy_script.py runs the real script against a throwaway git repo with a local bare origin, a fake `railway` on PATH that snapshots build-sha.txt at upload time, and a local http.server playing /health. AGENTS.md (CLAUDE.md is a symlink to it) gains a "Deploying the backend by CLI" section and a Verification pointer: use the script, never a bare `railway up`, and read /health before claiming a change is live. Co-Authored-By: Claude Fable 5.1 --- AGENTS.md | 17 ++ scripts/railway_deploy.sh | 208 +++++++++++++ tests/scripts/test_railway_deploy_script.py | 310 ++++++++++++++++++++ 3 files changed, 535 insertions(+) create mode 100755 scripts/railway_deploy.sh create mode 100644 tests/scripts/test_railway_deploy_script.py diff --git a/AGENTS.md b/AGENTS.md index 04ba02ea2..43eb776f0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -95,3 +95,20 @@ and `npm run build` for type errors. Full script list is in `frontend/package.json`. Some integration tests require PostgreSQL, Redis, Neo4j, Docker, or live services. If a command is blocked by local infrastructure or secrets, say so explicitly. + +Backend deploys go through `scripts/railway_deploy.sh`, never a bare `railway up`, and a backend change is live only once `/health` reports its SHA in `.build.commit_sha` (see "Deploying the backend by CLI"). + +## Deploying the backend by CLI + +The Railway service has no GitHub source and `.github/workflows/deploy.yml` is disabled, so a merge to `main` does not deploy. Production is deployed from a checkout: + +```bash +scripts/railway_deploy.sh +``` + +- Run it from a clean checkout whose HEAD is on `origin/main`. It refuses a dirty tree or an unmerged HEAD; `--allow-unmerged` overrides the second check on purpose. +- It stamps the HEAD SHA into `build-sha.txt` for the upload only and restores the tracked placeholder afterwards. Never commit a real SHA into that file. +- It runs `railway up --ci` with the service and environment from `RAILWAY_SERVICE_ID` / `RAILWAY_ENVIRONMENT_ID`, or from the linked project in `railway status`, and attaches the message ` @`. +- It then polls `${BACKEND_URL:-https://regengine-production.up.railway.app}/health` for up to 900 s and exits non-zero unless `.build.commit_sha` equals the SHA it shipped. Quote that line, not the upload, when reporting that a change is live. +- Never run a bare `railway up`: the image it ships cannot say which commit it is, and `/health` answers `build.commit_sha: null`. The older `scripts/deploy_railway.sh` pushes `main` and waits for a GitHub-triggered deploy that no longer exists; do not use it. +- Deploying is a production action. Do it only when Christopher asks for that deploy. diff --git a/scripts/railway_deploy.sh b/scripts/railway_deploy.sh new file mode 100755 index 000000000..b721a7c36 --- /dev/null +++ b/scripts/railway_deploy.sh @@ -0,0 +1,208 @@ +#!/usr/bin/env bash +# +# Deploy the RegEngine backend to Railway by CLI, carrying commit identity. +# +# Why this exists: the Railway service has no GitHub source and the GitHub +# Actions deploy workflow (.github/workflows/deploy.yml) is disabled, so +# production is deployed by running `railway up` from a checkout. A bare +# `railway up` ships an image that cannot say which commit it is: the upload +# carries no RAILWAY_GIT_COMMIT_SHA, and build-sha.txt (the stamp that +# services/shared/build_info.py reads) still holds its tracked placeholder. +# /health then answers `build.commit_sha: null` and nobody can tell from +# production which code is serving. +# +# This script: +# 1. refuses a dirty working tree, and refuses a HEAD that is not on +# origin/main unless --allow-unmerged is passed; +# 2. stamps the full HEAD SHA into build-sha.txt for the upload only, and +# restores the tracked placeholder afterwards, also on failure; +# 3. runs `railway up --ci` with an explicit service and environment and a +# deployment message of the form " @"; +# 4. polls ${BACKEND_URL}/health until .build.commit_sha equals the SHA it +# shipped, and exits non-zero if that does not happen within the budget. +# +# Usage: +# scripts/railway_deploy.sh [--allow-unmerged] +# +# Environment: +# RAILWAY_SERVICE_ID, RAILWAY_ENVIRONMENT_ID +# Deploy target. When either is unset it is read from `railway status`, +# which needs a linked project (`railway link`, `railway service`). +# BACKEND_URL +# Where to verify. Default https://regengine-production.up.railway.app +# RAILWAY_DEPLOY_TIMEOUT_SECONDS +# Rollout budget for the /health check. Default 900, matching deploy.yml. +# RAILWAY_DEPLOY_POLL_SECONDS +# Seconds between /health polls. Default 15, matching deploy.yml. +# +# Nothing here prints a token or the value of a Railway variable. + +set -euo pipefail + +usage() { + cat <<'EOF' +usage: scripts/railway_deploy.sh [--allow-unmerged] + +Deploy the current checkout to Railway with its commit SHA stamped into +build-sha.txt, then wait until /health reports that commit. + + --allow-unmerged deploy a HEAD that is not on origin/main + -h, --help show this help +EOF +} + +ALLOW_UNMERGED=0 +for arg in "$@"; do + case "$arg" in + --allow-unmerged) ALLOW_UNMERGED=1 ;; + -h|--help) usage; exit 0 ;; + *) echo "error: unknown argument '$arg'" >&2; usage >&2; exit 2 ;; + esac +done + +for tool in git railway curl jq; do + if ! command -v "$tool" >/dev/null 2>&1; then + echo "error: '$tool' is required but not on PATH" >&2 + exit 1 + fi +done + +REPO_ROOT="$(git rev-parse --show-toplevel)" +cd "$REPO_ROOT" +STAMP_FILE="build-sha.txt" +BACKEND_URL="${BACKEND_URL:-https://regengine-production.up.railway.app}" +TIMEOUT_SECONDS="${RAILWAY_DEPLOY_TIMEOUT_SECONDS:-900}" +POLL_SECONDS="${RAILWAY_DEPLOY_POLL_SECONDS:-15}" + +# ── 1. Preconditions ───────────────────────────────────────────────────────── +if ! git ls-files --error-unmatch "$STAMP_FILE" >/dev/null 2>&1; then + echo "error: ${STAMP_FILE} is not tracked. The Dockerfile copies it and build_info.py reads it." >&2 + exit 1 +fi + +if [ -n "$(git status --porcelain)" ]; then + echo "error: working tree is dirty. Commit or discard these changes before deploying;" >&2 + echo " railway up uploads the directory as it is, not the last commit." >&2 + git status --short >&2 + exit 1 +fi + +HEAD_SHA="$(git rev-parse --verify HEAD)" +SHORT_SHA="$(git rev-parse --short HEAD)" +SUBJECT="$(git log -1 --format=%s HEAD)" + +if [ "$ALLOW_UNMERGED" -eq 0 ]; then + if ! git fetch --quiet origin main; then + echo "error: could not fetch origin/main to confirm HEAD is merged (pass --allow-unmerged to skip this check)." >&2 + exit 1 + fi + if ! git merge-base --is-ancestor "$HEAD_SHA" origin/main; then + echo "error: HEAD ${SHORT_SHA} is not on origin/main." >&2 + echo " Merge first, or pass --allow-unmerged to deploy an unmerged commit on purpose." >&2 + exit 1 + fi + if [ "$HEAD_SHA" != "$(git rev-parse origin/main)" ]; then + echo "note: HEAD ${SHORT_SHA} is on origin/main but behind its tip; deploying the older commit." + fi +else + echo "note: --allow-unmerged set; not checking HEAD against origin/main." +fi + +# ── 2. Deploy target ───────────────────────────────────────────────────────── +SERVICE_ID="${RAILWAY_SERVICE_ID:-}" +ENVIRONMENT_ID="${RAILWAY_ENVIRONMENT_ID:-}" +SERVICE_SOURCE="RAILWAY_SERVICE_ID" +ENVIRONMENT_SOURCE="RAILWAY_ENVIRONMENT_ID" +if [ -z "$SERVICE_ID" ] || [ -z "$ENVIRONMENT_ID" ]; then + status_text="$(railway status)" || { + echo "error: 'railway status' failed. Link the project (railway link) or set RAILWAY_SERVICE_ID and RAILWAY_ENVIRONMENT_ID." >&2 + exit 1 + } + # `railway status` prints "Environment ID: " for the linked environment + # and, under "Linked service", " service ID: ". + if [ -z "$ENVIRONMENT_ID" ]; then + ENVIRONMENT_ID="$(printf '%s\n' "$status_text" | sed -n 's/^Environment ID:[[:space:]]*//p' | head -n 1 | tr -d '[:space:]')" + ENVIRONMENT_SOURCE="railway status" + fi + if [ -z "$SERVICE_ID" ]; then + SERVICE_ID="$(printf '%s\n' "$status_text" | sed -n 's/^[[:space:]]*service ID:[[:space:]]*//p' | head -n 1 | tr -d '[:space:]')" + SERVICE_SOURCE="railway status" + fi +fi +if [ -z "$SERVICE_ID" ] || [ -z "$ENVIRONMENT_ID" ]; then + echo "error: could not determine the Railway service and environment." >&2 + echo " Set RAILWAY_SERVICE_ID and RAILWAY_ENVIRONMENT_ID, or link a service with 'railway service'." >&2 + exit 1 +fi +echo "target: service from ${SERVICE_SOURCE}, environment from ${ENVIRONMENT_SOURCE}" + +# ── 3. Stamp and upload ────────────────────────────────────────────────────── +# The stamp is for the upload only. The tracked file keeps its placeholder so +# a real SHA is never committed; the trap restores it on every exit path. +# shellcheck disable=SC2329 # invoked by the EXIT trap below +restore_stamp() { + local rc=$? + if ! git checkout --quiet -- "$STAMP_FILE"; then + echo "warning: could not restore ${STAMP_FILE}; run: git checkout -- ${STAMP_FILE}" >&2 + fi + exit "$rc" +} +trap restore_stamp EXIT + +printf '%s\n' "$HEAD_SHA" > "$STAMP_FILE" +echo "stamped ${STAMP_FILE} with ${HEAD_SHA}" + +MESSAGE="${SUBJECT} @${SHORT_SHA}" +echo "uploading: railway up --ci --message '${MESSAGE}'" +railway up --ci --service "$SERVICE_ID" --environment "$ENVIRONMENT_ID" --message "$MESSAGE" + +git checkout --quiet -- "$STAMP_FILE" +echo "restored ${STAMP_FILE} to its tracked placeholder" + +# ── 4. Verify the deployed commit is the one we shipped ────────────────────── +# Mirrors the "Verify the deployed commit is the one we shipped" step in +# .github/workflows/deploy.yml. `railway up --ci` returns once the build is +# done, not once the new image is serving. railway.toml sets +# restartPolicyType = "ON_FAILURE" with a /readiness healthcheck, so a rollout +# that fails its healthcheck reverts to the previous image after the upload +# already looked fine. Asserting identity (the served commit) instead of +# behaviour is what closes that hole. +EXPECTED_SHA="$HEAD_SHA" +deadline=$(( SECONDS + TIMEOUT_SECONDS )) +attempt=0 +last_sha="" +last_code="" + +echo "waiting up to ${TIMEOUT_SECONDS}s for ${BACKEND_URL}/health to report ${EXPECTED_SHA}" +while [ "$SECONDS" -lt "$deadline" ]; do + attempt=$(( attempt + 1 )) + body=$(mktemp) + last_code=$(curl -sS --max-time 20 -o "$body" -w '%{http_code}' "${BACKEND_URL}/health" || true) + last_code="${last_code:-000}" + + if [ "$last_code" = "200" ]; then + # `// empty` also covers a payload with no build key at all, which is + # what an image predating build stamping returns. + last_sha=$(jq -r '.build.commit_sha // empty' "$body" 2>/dev/null || true) + if [ "$last_sha" = "$EXPECTED_SHA" ]; then + rm -f "$body" + echo "Deployed commit matches: ${EXPECTED_SHA}" + exit 0 + fi + fi + rm -f "$body" + + echo "attempt ${attempt}: status=${last_code} deployed_sha=${last_sha:-(none)} expected=${EXPECTED_SHA}" + sleep "$POLL_SECONDS" +done + +echo "error: deploy did not converge within ${TIMEOUT_SECONDS}s. /health last reported status=${last_code} commit_sha=${last_sha:-(none)}, expected ${EXPECTED_SHA}." >&2 +# Three distinct causes, three distinct messages, as in deploy.yml. +if [ "$last_code" != "200" ]; then + echo "error: /health never returned 200 (last status ${last_code}). The service is not serving; check the Railway deploy logs for a crashed or unhealthy rollout." >&2 +elif [ -z "$last_sha" ]; then + echo "error: /health answered but carried no commit_sha. Either the rollout reverted to an image predating build stamping, or build-sha.txt did not reach the image." >&2 +else + echo "error: a different commit is serving. The rollout reverted (railway.toml restartPolicyType=ON_FAILURE) or another deploy raced this one." >&2 +fi +exit 1 diff --git a/tests/scripts/test_railway_deploy_script.py b/tests/scripts/test_railway_deploy_script.py new file mode 100644 index 000000000..3788cc8af --- /dev/null +++ b/tests/scripts/test_railway_deploy_script.py @@ -0,0 +1,310 @@ +"""scripts/railway_deploy.sh must ship commit identity and verify the rollout. + +Production is deployed by running `railway up` from a checkout. A bare upload +leaves /health answering `build.commit_sha: null`, because the tracked +build-sha.txt placeholder is what reaches the image. The script under test +stamps HEAD into build-sha.txt for the upload only, restores the placeholder, +and polls /health until the served commit is the one it shipped. + +These tests run the real script against a throwaway git repository with a +local bare "origin", a fake `railway` executable placed first on PATH, and a +local HTTP server standing in for /health. +""" + +from __future__ import annotations + +import http.server +import json +import os +import socket +import subprocess +import threading +from dataclasses import dataclass, field +from pathlib import Path + +import pytest + +_REPO = Path(__file__).resolve().parents[2] +_SCRIPT = _REPO / "scripts" / "railway_deploy.sh" +_PLACEHOLDER = "unknown\n" +_SUBJECT = "feat: initial deployable commit" + +# Records each invocation as one argument per line and, for `up`, snapshots +# build-sha.txt exactly as it was at upload time. `status` prints the shape +# the real CLI prints for a linked project (ids are fakes). +_FAKE_RAILWAY = """#!/usr/bin/env bash +set -euo pipefail +n=$(find "$FAKE_RAILWAY_LOG_DIR" -name '*.args' | wc -l | tr -d ' ') +printf '%s\\n' "$@" > "$FAKE_RAILWAY_LOG_DIR/$n.args" +case "${1:-}" in + status) + cat <<'STATUS' +Workspace: Example Workspace + +Project: RegEngine-Prod +Project ID: 00000000-0000-0000-0000-000000000001 + +Environment: production +Environment ID: env-from-status + +Linked service + +RegEngine + status: Online + deployment ID: 00000000-0000-0000-0000-000000000002 + service ID: svc-from-status +STATUS + ;; + up) + cp build-sha.txt "$FAKE_RAILWAY_LOG_DIR/$n.stamp" + ;; +esac +""" + + +def _git(cwd: Path, *args: str) -> str: + return subprocess.run( + ["git", *args], + cwd=cwd, + check=True, + capture_output=True, + text=True, + env=_hermetic_git_env(), + ).stdout.strip() + + +def _hermetic_git_env() -> dict[str, str]: + env = os.environ.copy() + env["GIT_CONFIG_GLOBAL"] = os.devnull + env["GIT_CONFIG_NOSYSTEM"] = "1" + return env + + +class _HealthState: + def __init__(self) -> None: + self.sha: str | None = None + self.requests = 0 + self.url = "" + + +@dataclass +class _Harness: + repo: Path + bin_dir: Path + log_dir: Path + health: _HealthState + env_overrides: dict[str, str | None] = field(default_factory=dict) + + def head(self) -> str: + return _git(self.repo, "rev-parse", "HEAD") + + def short_head(self) -> str: + return _git(self.repo, "rev-parse", "--short", "HEAD") + + def run(self, *args: str, **overrides: str | None) -> subprocess.CompletedProcess[str]: + env = _hermetic_git_env() + for key in [k for k in env if k.startswith("RAILWAY_")]: + env.pop(key) + env.update( + { + "PATH": f"{self.bin_dir}{os.pathsep}{env['PATH']}", + "FAKE_RAILWAY_LOG_DIR": str(self.log_dir), + "BACKEND_URL": self.health.url, + "RAILWAY_SERVICE_ID": "svc-from-env", + "RAILWAY_ENVIRONMENT_ID": "env-from-env", + # Keep the real 900 s / 15 s defaults out of the test run. + "RAILWAY_DEPLOY_TIMEOUT_SECONDS": "3", + "RAILWAY_DEPLOY_POLL_SECONDS": "0.2", + # A token in the environment must never reach the output. + "RAILWAY_TOKEN": "fake-token-must-not-be-printed", + } + ) + for key, value in {**self.env_overrides, **overrides}.items(): + if value is None: + env.pop(key, None) + else: + env[key] = value + return subprocess.run( + ["bash", str(_SCRIPT), *args], + cwd=self.repo, + env=env, + capture_output=True, + text=True, + timeout=60, + ) + + def calls(self) -> list[list[str]]: + files = sorted(self.log_dir.glob("*.args"), key=lambda p: int(p.stem)) + return [f.read_text(encoding="utf-8").splitlines() for f in files] + + def stamp_at_upload(self) -> str: + (stamp,) = self.log_dir.glob("*.stamp") + return stamp.read_text(encoding="utf-8") + + def stamp_file(self) -> str: + return (self.repo / "build-sha.txt").read_text(encoding="utf-8") + + +@pytest.fixture +def health() -> _HealthState: + state = _HealthState() + + class Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 - http.server API + state.requests += 1 + if self.path != "/health": + self.send_response(404) + self.end_headers() + return + body = json.dumps({"status": "ok", "build": {"commit_sha": state.sha}}).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *_args: object) -> None: + return + + server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), Handler) + threading.Thread(target=server.serve_forever, daemon=True).start() + state.url = f"http://127.0.0.1:{server.server_address[1]}" + try: + yield state + finally: + server.shutdown() + server.server_close() + + +@pytest.fixture +def harness(tmp_path: Path, health: _HealthState) -> _Harness: + work = tmp_path / "work" + work.mkdir() + _git(work, "init", "-q", "-b", "main") + _git(work, "config", "user.email", "deploy-test@example.com") + _git(work, "config", "user.name", "Deploy Test") + _git(work, "config", "commit.gpgsign", "false") + (work / "build-sha.txt").write_text(_PLACEHOLDER, encoding="utf-8") + (work / "app.txt").write_text("v1\n", encoding="utf-8") + _git(work, "add", ".") + _git(work, "commit", "-q", "-m", _SUBJECT) + + origin = tmp_path / "origin.git" + _git(tmp_path, "init", "-q", "--bare", "-b", "main", str(origin)) + _git(work, "remote", "add", "origin", str(origin)) + _git(work, "push", "-q", "-u", "origin", "main") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + fake = bin_dir / "railway" + fake.write_text(_FAKE_RAILWAY, encoding="utf-8") + fake.chmod(0o755) + log_dir = tmp_path / "railway-calls" + log_dir.mkdir() + return _Harness(repo=work, bin_dir=bin_dir, log_dir=log_dir, health=health) + + +def _commit_unpushed_change(harness: _Harness) -> None: + (harness.repo / "app.txt").write_text("v2\n", encoding="utf-8") + _git(harness.repo, "commit", "-q", "-am", "feat: not on origin/main yet") + + +def test_refuses_a_dirty_working_tree(harness: _Harness) -> None: + (harness.repo / "app.txt").write_text("edited but not committed\n", encoding="utf-8") + + result = harness.run() + + assert result.returncode != 0 + assert "working tree is dirty" in result.stderr + assert harness.calls() == [], "railway must not be invoked for a dirty tree" + assert harness.stamp_file() == _PLACEHOLDER + + +def test_refuses_a_head_that_is_not_on_origin_main(harness: _Harness) -> None: + _commit_unpushed_change(harness) + + result = harness.run() + + assert result.returncode != 0 + assert "not on origin/main" in result.stderr + assert harness.calls() == [] + assert harness.stamp_file() == _PLACEHOLDER + + +def test_allow_unmerged_deploys_an_unpushed_head(harness: _Harness) -> None: + _commit_unpushed_change(harness) + harness.health.sha = harness.head() + + result = harness.run("--allow-unmerged") + + assert result.returncode == 0, result.stderr + assert [call[0] for call in harness.calls()] == ["up"] + assert harness.stamp_at_upload() == harness.head() + "\n" + + +def test_stamps_head_sha_for_the_upload_and_restores_the_placeholder(harness: _Harness) -> None: + head = harness.head() + harness.health.sha = head + + result = harness.run() + + assert result.returncode == 0, result.stderr + (up,) = harness.calls() + assert up == [ + "up", + "--ci", + "--service", + "svc-from-env", + "--environment", + "env-from-env", + "--message", + f"{_SUBJECT} @{harness.short_head()}", + ] + assert harness.stamp_at_upload() == head + "\n", "the full SHA must be in build-sha.txt when railway up runs" + assert harness.stamp_file() == _PLACEHOLDER, "the tracked placeholder must be restored after the upload" + assert _git(harness.repo, "status", "--porcelain") == "" + assert f"Deployed commit matches: {head}" in result.stdout + output = result.stdout + result.stderr + assert "fake-token-must-not-be-printed" not in output + assert "svc-from-env" not in output and "env-from-env" not in output + + +def test_reads_service_and_environment_from_railway_status_when_unset(harness: _Harness) -> None: + harness.health.sha = harness.head() + + result = harness.run(RAILWAY_SERVICE_ID=None, RAILWAY_ENVIRONMENT_ID=None) + + assert result.returncode == 0, result.stderr + status, up = harness.calls() + assert status == ["status"] + assert up[:6] == ["up", "--ci", "--service", "svc-from-status", "--environment", "env-from-status"] + assert "svc-from-status" not in result.stdout + result.stderr + + +def test_fails_when_health_never_reports_the_shipped_sha(harness: _Harness) -> None: + stale = "0" * 40 + harness.health.sha = stale + + result = harness.run() + + assert result.returncode != 0 + assert harness.health.requests >= 2, "the script must keep polling until the budget is spent" + assert "deploy did not converge" in result.stderr + assert "status=200" in result.stderr + assert f"commit_sha={stale}" in result.stderr + assert "a different commit is serving" in result.stderr + assert harness.stamp_file() == _PLACEHOLDER, "the placeholder must be restored even when verification fails" + assert _git(harness.repo, "status", "--porcelain") == "" + + +def test_fails_when_health_is_unreachable(harness: _Harness) -> None: + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + closed_port = probe.getsockname()[1] + + result = harness.run(BACKEND_URL=f"http://127.0.0.1:{closed_port}") + + assert result.returncode != 0 + assert "status=000" in result.stderr + assert "never returned 200" in result.stderr + assert harness.stamp_file() == _PLACEHOLDER From d6858c77cac23cf7213c95c035d89b878afad0e9 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Fri, 30 Oct 2026 00:39:25 -0700 Subject: [PATCH 2/6] ci(deploy): refuse worktrees, bind the health URL to the target, pass --project Three guards found by review and by a sibling session's deploy attempt: - `railway up` fails with "Indexing... prefix not found" when .git is a worktree pointer file (observed 2026-10-29), which is where most agent sessions run. Refuse before stamping and say to use the primary checkout. - The production /health default could verify the wrong deployment when the target is staging: a hand-picked RAILWAY_ENVIRONMENT_ID, or a linked non-production environment, now requires an explicit BACKEND_URL. - Check the exit status of `git status --porcelain` before reading its output, so a failed inspection never reads as a clean tree. - Pass `railway up --project` when RAILWAY_PROJECT_ID is set, as deploy.yml does; otherwise the linked project is used. The "target:" line names the source of each id, never its value (a test caught the value leaking). AGENTS.md now says deploy.yml is disabled in GitHub Actions although its YAML still declares a push trigger, which is what a reviewer tripped on. Co-Authored-By: Claude Fable 5.1 --- AGENTS.md | 6 ++- scripts/railway_deploy.sh | 59 ++++++++++++++++++--- tests/scripts/test_railway_deploy_script.py | 58 ++++++++++++++++++-- 3 files changed, 109 insertions(+), 14 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 43eb776f0..6799bea99 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -100,7 +100,7 @@ Backend deploys go through `scripts/railway_deploy.sh`, never a bare `railway up ## Deploying the backend by CLI -The Railway service has no GitHub source and `.github/workflows/deploy.yml` is disabled, so a merge to `main` does not deploy. Production is deployed from a checkout: +The Railway service has no GitHub source, and `.github/workflows/deploy.yml` is disabled in GitHub Actions (its YAML still declares a `push` trigger, but a disabled workflow never runs), so a merge to `main` does not deploy. Production is deployed from a checkout: ```bash scripts/railway_deploy.sh @@ -108,7 +108,9 @@ scripts/railway_deploy.sh - Run it from a clean checkout whose HEAD is on `origin/main`. It refuses a dirty tree or an unmerged HEAD; `--allow-unmerged` overrides the second check on purpose. - It stamps the HEAD SHA into `build-sha.txt` for the upload only and restores the tracked placeholder afterwards. Never commit a real SHA into that file. -- It runs `railway up --ci` with the service and environment from `RAILWAY_SERVICE_ID` / `RAILWAY_ENVIRONMENT_ID`, or from the linked project in `railway status`, and attaches the message ` @`. +- It runs `railway up --ci` with the service and environment from `RAILWAY_SERVICE_ID` / `RAILWAY_ENVIRONMENT_ID`, or from the linked project in `railway status`, plus `--project` when `RAILWAY_PROJECT_ID` is set, and attaches the message ` @`. +- Run it from the primary checkout, not a git worktree: `railway up` cannot index a checkout whose `.git` is a file, and the script refuses one. +- For any target other than the linked production environment, set `BACKEND_URL` to that environment's public URL. The script refuses a hand-picked `RAILWAY_ENVIRONMENT_ID` or a linked non-production environment without it, because the production default could pass while the real target never served the upload. - It then polls `${BACKEND_URL:-https://regengine-production.up.railway.app}/health` for up to 900 s and exits non-zero unless `.build.commit_sha` equals the SHA it shipped. Quote that line, not the upload, when reporting that a change is live. - Never run a bare `railway up`: the image it ships cannot say which commit it is, and `/health` answers `build.commit_sha: null`. The older `scripts/deploy_railway.sh` pushes `main` and waits for a GitHub-triggered deploy that no longer exists; do not use it. - Deploying is a production action. Do it only when Christopher asks for that deploy. diff --git a/scripts/railway_deploy.sh b/scripts/railway_deploy.sh index b721a7c36..181fb58f6 100755 --- a/scripts/railway_deploy.sh +++ b/scripts/railway_deploy.sh @@ -3,8 +3,10 @@ # Deploy the RegEngine backend to Railway by CLI, carrying commit identity. # # Why this exists: the Railway service has no GitHub source and the GitHub -# Actions deploy workflow (.github/workflows/deploy.yml) is disabled, so -# production is deployed by running `railway up` from a checkout. A bare +# Actions deploy workflow (.github/workflows/deploy.yml) is disabled in the +# Actions UI (its YAML still declares a push trigger, but a disabled workflow +# never runs), so production is deployed by running `railway up` from a +# checkout. A bare # `railway up` ships an image that cannot say which commit it is: the upload # carries no RAILWAY_GIT_COMMIT_SHA, and build-sha.txt (the stamp that # services/shared/build_info.py reads) still holds its tracked placeholder. @@ -28,8 +30,15 @@ # RAILWAY_SERVICE_ID, RAILWAY_ENVIRONMENT_ID # Deploy target. When either is unset it is read from `railway status`, # which needs a linked project (`railway link`, `railway service`). +# RAILWAY_PROJECT_ID +# Passed as `railway up --project` when set, as deploy.yml does. +# Otherwise the linked project is used. # BACKEND_URL -# Where to verify. Default https://regengine-production.up.railway.app +# Where to verify. Default https://regengine-production.up.railway.app, +# used only for the production environment: a target chosen by hand +# (RAILWAY_ENVIRONMENT_ID) or a linked non-production environment must +# set BACKEND_URL explicitly, or the check could pass against production +# while the real target never served the upload. # RAILWAY_DEPLOY_TIMEOUT_SECONDS # Rollout budget for the /health check. Default 900, matching deploy.yml. # RAILWAY_DEPLOY_POLL_SECONDS @@ -70,6 +79,17 @@ done REPO_ROOT="$(git rev-parse --show-toplevel)" cd "$REPO_ROOT" STAMP_FILE="build-sha.txt" + +# `railway up` indexes the upload relative to the git toplevel and fails with +# "Indexing... prefix not found" when .git is a worktree pointer file rather +# than a directory (observed 2026-10-29). Refuse before touching anything. +if [ ! -d "$REPO_ROOT/.git" ]; then + echo "error: ${REPO_ROOT} is a git worktree (.git is a file). railway up cannot index a worktree;" >&2 + echo " run this from the primary checkout, whose .git is a directory." >&2 + exit 1 +fi +BACKEND_URL_SET=1 +[ -n "${BACKEND_URL:-}" ] || BACKEND_URL_SET=0 BACKEND_URL="${BACKEND_URL:-https://regengine-production.up.railway.app}" TIMEOUT_SECONDS="${RAILWAY_DEPLOY_TIMEOUT_SECONDS:-900}" POLL_SECONDS="${RAILWAY_DEPLOY_POLL_SECONDS:-15}" @@ -80,10 +100,16 @@ if ! git ls-files --error-unmatch "$STAMP_FILE" >/dev/null 2>&1; then exit 1 fi -if [ -n "$(git status --porcelain)" ]; then +# A failed inspection must not read as a clean tree, so check the exit +# status before the output. +dirty="$(git status --porcelain)" || { + echo "error: 'git status' failed; refusing to treat the checkout as clean." >&2 + exit 1 +} +if [ -n "$dirty" ]; then echo "error: working tree is dirty. Commit or discard these changes before deploying;" >&2 echo " railway up uploads the directory as it is, not the last commit." >&2 - git status --short >&2 + printf '%s\n' "$dirty" >&2 exit 1 fi @@ -123,6 +149,12 @@ if [ -z "$SERVICE_ID" ] || [ -z "$ENVIRONMENT_ID" ]; then if [ -z "$ENVIRONMENT_ID" ]; then ENVIRONMENT_ID="$(printf '%s\n' "$status_text" | sed -n 's/^Environment ID:[[:space:]]*//p' | head -n 1 | tr -d '[:space:]')" ENVIRONMENT_SOURCE="railway status" + environment_name="$(printf '%s\n' "$status_text" | sed -n 's/^Environment:[[:space:]]*//p' | head -n 1 | tr -d '[:space:]')" + if [ "$BACKEND_URL_SET" -eq 0 ] && [ "$environment_name" != "production" ]; then + echo "error: the linked environment is '${environment_name:-unknown}', not production, and BACKEND_URL is unset." >&2 + echo " Set BACKEND_URL to that environment's public URL so the health check verifies the right deployment." >&2 + exit 1 + fi fi if [ -z "$SERVICE_ID" ]; then SERVICE_ID="$(printf '%s\n' "$status_text" | sed -n 's/^[[:space:]]*service ID:[[:space:]]*//p' | head -n 1 | tr -d '[:space:]')" @@ -134,7 +166,18 @@ if [ -z "$SERVICE_ID" ] || [ -z "$ENVIRONMENT_ID" ]; then echo " Set RAILWAY_SERVICE_ID and RAILWAY_ENVIRONMENT_ID, or link a service with 'railway service'." >&2 exit 1 fi -echo "target: service from ${SERVICE_SOURCE}, environment from ${ENVIRONMENT_SOURCE}" +if [ "$ENVIRONMENT_SOURCE" = "RAILWAY_ENVIRONMENT_ID" ] && [ "$BACKEND_URL_SET" -eq 0 ]; then + echo "error: RAILWAY_ENVIRONMENT_ID selects the environment by hand but BACKEND_URL is unset." >&2 + echo " Set BACKEND_URL to that environment's public URL so the health check verifies the right deployment." >&2 + exit 1 +fi +PROJECT_ARGS=() +PROJECT_SOURCE="the linked project" +if [ -n "${RAILWAY_PROJECT_ID:-}" ]; then + PROJECT_ARGS=(--project "$RAILWAY_PROJECT_ID") + PROJECT_SOURCE="RAILWAY_PROJECT_ID" +fi +echo "target: service from ${SERVICE_SOURCE}, environment from ${ENVIRONMENT_SOURCE}, project from ${PROJECT_SOURCE}" # ── 3. Stamp and upload ────────────────────────────────────────────────────── # The stamp is for the upload only. The tracked file keeps its placeholder so @@ -154,7 +197,9 @@ echo "stamped ${STAMP_FILE} with ${HEAD_SHA}" MESSAGE="${SUBJECT} @${SHORT_SHA}" echo "uploading: railway up --ci --message '${MESSAGE}'" -railway up --ci --service "$SERVICE_ID" --environment "$ENVIRONMENT_ID" --message "$MESSAGE" +# ${arr[@]+"${arr[@]}"} expands to nothing when the array is empty and stays +# safe under `set -u` on every bash, including macOS /bin/bash 3.2. +railway up --ci ${PROJECT_ARGS[@]+"${PROJECT_ARGS[@]}"} --service "$SERVICE_ID" --environment "$ENVIRONMENT_ID" --message "$MESSAGE" git checkout --quiet -- "$STAMP_FILE" echo "restored ${STAMP_FILE} to its tracked placeholder" diff --git a/tests/scripts/test_railway_deploy_script.py b/tests/scripts/test_railway_deploy_script.py index 3788cc8af..7e8b5e015 100644 --- a/tests/scripts/test_railway_deploy_script.py +++ b/tests/scripts/test_railway_deploy_script.py @@ -38,13 +38,13 @@ printf '%s\\n' "$@" > "$FAKE_RAILWAY_LOG_DIR/$n.args" case "${1:-}" in status) - cat <<'STATUS' + cat < str: def short_head(self) -> str: return _git(self.repo, "rev-parse", "--short", "HEAD") - def run(self, *args: str, **overrides: str | None) -> subprocess.CompletedProcess[str]: + def run(self, *args: str, cwd: Path | None = None, **overrides: str | None) -> subprocess.CompletedProcess[str]: env = _hermetic_git_env() for key in [k for k in env if k.startswith("RAILWAY_")]: env.pop(key) @@ -126,7 +126,7 @@ def run(self, *args: str, **overrides: str | None) -> subprocess.CompletedProces env[key] = value return subprocess.run( ["bash", str(_SCRIPT), *args], - cwd=self.repo, + cwd=cwd or self.repo, env=env, capture_output=True, text=True, @@ -220,6 +220,21 @@ def test_refuses_a_dirty_working_tree(harness: _Harness) -> None: assert harness.stamp_file() == _PLACEHOLDER +def test_refuses_to_run_from_a_git_worktree(harness: _Harness) -> None: + # `railway up` fails with "Indexing... prefix not found" when .git is a + # worktree pointer file, which is where most agent sessions run. + worktree = harness.repo.parent / "worktree" + _git(harness.repo, "worktree", "add", "-q", str(worktree), "-b", "deploy-from-worktree") + harness.health.sha = harness.head() + + result = harness.run(cwd=worktree) + + assert result.returncode != 0 + assert "git worktree" in result.stderr + assert harness.calls() == [] + assert (worktree / "build-sha.txt").read_text(encoding="utf-8") == _PLACEHOLDER + + def test_refuses_a_head_that_is_not_on_origin_main(harness: _Harness) -> None: _commit_unpushed_change(harness) @@ -246,13 +261,15 @@ def test_stamps_head_sha_for_the_upload_and_restores_the_placeholder(harness: _H head = harness.head() harness.health.sha = head - result = harness.run() + result = harness.run(RAILWAY_PROJECT_ID="proj-from-env") assert result.returncode == 0, result.stderr (up,) = harness.calls() assert up == [ "up", "--ci", + "--project", + "proj-from-env", "--service", "svc-from-env", "--environment", @@ -267,6 +284,7 @@ def test_stamps_head_sha_for_the_upload_and_restores_the_placeholder(harness: _H output = result.stdout + result.stderr assert "fake-token-must-not-be-printed" not in output assert "svc-from-env" not in output and "env-from-env" not in output + assert "proj-from-env" not in output def test_reads_service_and_environment_from_railway_status_when_unset(harness: _Harness) -> None: @@ -278,9 +296,39 @@ def test_reads_service_and_environment_from_railway_status_when_unset(harness: _ status, up = harness.calls() assert status == ["status"] assert up[:6] == ["up", "--ci", "--service", "svc-from-status", "--environment", "env-from-status"] + assert "--project" not in up assert "svc-from-status" not in result.stdout + result.stderr +def test_requires_backend_url_when_the_environment_is_chosen_by_hand(harness: _Harness) -> None: + # RAILWAY_ENVIRONMENT_ID may point at staging; the production default URL + # would then verify the wrong deployment and could pass by accident. + harness.health.sha = harness.head() + + result = harness.run(BACKEND_URL=None) + + assert result.returncode != 0 + assert "BACKEND_URL is unset" in result.stderr + assert harness.calls() == [] + assert harness.stamp_file() == _PLACEHOLDER + + +def test_refuses_a_non_production_linked_environment_without_backend_url(harness: _Harness) -> None: + harness.health.sha = harness.head() + + result = harness.run( + RAILWAY_SERVICE_ID=None, + RAILWAY_ENVIRONMENT_ID=None, + BACKEND_URL=None, + FAKE_RAILWAY_ENV_NAME="staging", + ) + + assert result.returncode != 0 + assert "'staging', not production" in result.stderr + assert harness.calls() == [["status"]] + assert harness.stamp_file() == _PLACEHOLDER + + def test_fails_when_health_never_reports_the_shipped_sha(harness: _Harness) -> None: stale = "0" * 40 harness.health.sha = stale From b8509e0b2d733fbfe88ff9e592865548e08371ca Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Fri, 30 Oct 2026 00:50:05 -0700 Subject: [PATCH 3/6] ci(deploy): use the default health URL only for the exact linked production target The production /health default was accepted for any linked environment named "production", so a checkout linked to another project or service could deploy there and still verify against RegEngine production. The default is now tied to project RegEngine-Prod, environment production, service RegEngine as `railway status` reports them, and only when both ids come from `railway status`. Any id picked by hand, or any other linked target, must set BACKEND_URL. Tests cover a mismatched project, environment and service, and the matching production target through a fake curl so the default URL path runs without the network. Co-Authored-By: Claude Fable 5.1 --- AGENTS.md | 2 +- scripts/railway_deploy.sh | 48 ++++++++++------ tests/scripts/test_railway_deploy_script.py | 63 ++++++++++++++++++--- 3 files changed, 88 insertions(+), 25 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6799bea99..bdcd587d5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,7 +110,7 @@ scripts/railway_deploy.sh - It stamps the HEAD SHA into `build-sha.txt` for the upload only and restores the tracked placeholder afterwards. Never commit a real SHA into that file. - It runs `railway up --ci` with the service and environment from `RAILWAY_SERVICE_ID` / `RAILWAY_ENVIRONMENT_ID`, or from the linked project in `railway status`, plus `--project` when `RAILWAY_PROJECT_ID` is set, and attaches the message ` @`. - Run it from the primary checkout, not a git worktree: `railway up` cannot index a checkout whose `.git` is a file, and the script refuses one. -- For any target other than the linked production environment, set `BACKEND_URL` to that environment's public URL. The script refuses a hand-picked `RAILWAY_ENVIRONMENT_ID` or a linked non-production environment without it, because the production default could pass while the real target never served the upload. +- The default health URL belongs to one target: project RegEngine-Prod, environment production, service RegEngine, as `railway status` reports them. For anything else (an id picked by hand, or another linked project, environment or service) set `BACKEND_URL` to that target's public URL; the script refuses otherwise, because the production default could pass while the real target never served the upload. - It then polls `${BACKEND_URL:-https://regengine-production.up.railway.app}/health` for up to 900 s and exits non-zero unless `.build.commit_sha` equals the SHA it shipped. Quote that line, not the upload, when reporting that a change is live. - Never run a bare `railway up`: the image it ships cannot say which commit it is, and `/health` answers `build.commit_sha: null`. The older `scripts/deploy_railway.sh` pushes `main` and waits for a GitHub-triggered deploy that no longer exists; do not use it. - Deploying is a production action. Do it only when Christopher asks for that deploy. diff --git a/scripts/railway_deploy.sh b/scripts/railway_deploy.sh index 181fb58f6..6c4aa61b7 100755 --- a/scripts/railway_deploy.sh +++ b/scripts/railway_deploy.sh @@ -34,11 +34,12 @@ # Passed as `railway up --project` when set, as deploy.yml does. # Otherwise the linked project is used. # BACKEND_URL -# Where to verify. Default https://regengine-production.up.railway.app, -# used only for the production environment: a target chosen by hand -# (RAILWAY_ENVIRONMENT_ID) or a linked non-production environment must -# set BACKEND_URL explicitly, or the check could pass against production -# while the real target never served the upload. +# Where to verify. The default, https://regengine-production.up.railway.app, +# belongs to one target (project RegEngine-Prod, environment production, +# service RegEngine) and is used only when `railway status` reports that +# exact linked target. Any id picked by hand, or any other linked target, +# must set BACKEND_URL explicitly, or the check could pass against +# production while the real target never served the upload. # RAILWAY_DEPLOY_TIMEOUT_SECONDS # Rollout budget for the /health check. Default 900, matching deploy.yml. # RAILWAY_DEPLOY_POLL_SECONDS @@ -88,9 +89,15 @@ if [ ! -d "$REPO_ROOT/.git" ]; then echo " run this from the primary checkout, whose .git is a directory." >&2 exit 1 fi +# The default URL and the target it belongs to. Changing one means changing +# the others; the script refuses the default for any other target. +DEFAULT_BACKEND_URL="https://regengine-production.up.railway.app" +DEFAULT_TARGET_PROJECT="RegEngine-Prod" +DEFAULT_TARGET_ENVIRONMENT="production" +DEFAULT_TARGET_SERVICE="RegEngine" BACKEND_URL_SET=1 [ -n "${BACKEND_URL:-}" ] || BACKEND_URL_SET=0 -BACKEND_URL="${BACKEND_URL:-https://regengine-production.up.railway.app}" +BACKEND_URL="${BACKEND_URL:-$DEFAULT_BACKEND_URL}" TIMEOUT_SECONDS="${RAILWAY_DEPLOY_TIMEOUT_SECONDS:-900}" POLL_SECONDS="${RAILWAY_DEPLOY_POLL_SECONDS:-15}" @@ -149,12 +156,6 @@ if [ -z "$SERVICE_ID" ] || [ -z "$ENVIRONMENT_ID" ]; then if [ -z "$ENVIRONMENT_ID" ]; then ENVIRONMENT_ID="$(printf '%s\n' "$status_text" | sed -n 's/^Environment ID:[[:space:]]*//p' | head -n 1 | tr -d '[:space:]')" ENVIRONMENT_SOURCE="railway status" - environment_name="$(printf '%s\n' "$status_text" | sed -n 's/^Environment:[[:space:]]*//p' | head -n 1 | tr -d '[:space:]')" - if [ "$BACKEND_URL_SET" -eq 0 ] && [ "$environment_name" != "production" ]; then - echo "error: the linked environment is '${environment_name:-unknown}', not production, and BACKEND_URL is unset." >&2 - echo " Set BACKEND_URL to that environment's public URL so the health check verifies the right deployment." >&2 - exit 1 - fi fi if [ -z "$SERVICE_ID" ]; then SERVICE_ID="$(printf '%s\n' "$status_text" | sed -n 's/^[[:space:]]*service ID:[[:space:]]*//p' | head -n 1 | tr -d '[:space:]')" @@ -166,10 +167,25 @@ if [ -z "$SERVICE_ID" ] || [ -z "$ENVIRONMENT_ID" ]; then echo " Set RAILWAY_SERVICE_ID and RAILWAY_ENVIRONMENT_ID, or link a service with 'railway service'." >&2 exit 1 fi -if [ "$ENVIRONMENT_SOURCE" = "RAILWAY_ENVIRONMENT_ID" ] && [ "$BACKEND_URL_SET" -eq 0 ]; then - echo "error: RAILWAY_ENVIRONMENT_ID selects the environment by hand but BACKEND_URL is unset." >&2 - echo " Set BACKEND_URL to that environment's public URL so the health check verifies the right deployment." >&2 - exit 1 +if [ "$BACKEND_URL_SET" -eq 0 ]; then + # The default URL is tied to one target. Use it only when `railway status` + # reports that exact linked target; a hand-picked id, or another linked + # project, environment or service, must say where to verify. + if [ "$SERVICE_SOURCE" != "railway status" ] || [ "$ENVIRONMENT_SOURCE" != "railway status" ]; then + echo "error: RAILWAY_SERVICE_ID / RAILWAY_ENVIRONMENT_ID pick the target by hand but BACKEND_URL is unset." >&2 + echo " Set BACKEND_URL to that target's public URL so the health check verifies the right deployment." >&2 + exit 1 + fi + project_name="$(printf '%s\n' "$status_text" | sed -n 's/^Project:[[:space:]]*//p' | head -n 1 | sed 's/[[:space:]]*$//')" + environment_name="$(printf '%s\n' "$status_text" | sed -n 's/^Environment:[[:space:]]*//p' | head -n 1 | sed 's/[[:space:]]*$//')" + # The linked service's name is the first non-empty line after "Linked service". + service_name="$(printf '%s\n' "$status_text" | awk '/^Linked service/ { linked = 1; next } linked && NF { print; exit }' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" + if [ "$project_name" != "$DEFAULT_TARGET_PROJECT" ] || [ "$environment_name" != "$DEFAULT_TARGET_ENVIRONMENT" ] || [ "$service_name" != "$DEFAULT_TARGET_SERVICE" ]; then + echo "error: the linked target is project '${project_name:-unknown}', environment '${environment_name:-unknown}', service '${service_name:-unknown}'," >&2 + echo " but the default BACKEND_URL belongs to ${DEFAULT_TARGET_PROJECT} / ${DEFAULT_TARGET_ENVIRONMENT} / ${DEFAULT_TARGET_SERVICE}." >&2 + echo " Set BACKEND_URL to this target's public URL so the health check verifies the right deployment." >&2 + exit 1 + fi fi PROJECT_ARGS=() PROJECT_SOURCE="the linked project" diff --git a/tests/scripts/test_railway_deploy_script.py b/tests/scripts/test_railway_deploy_script.py index 7e8b5e015..c2f2fe4e3 100644 --- a/tests/scripts/test_railway_deploy_script.py +++ b/tests/scripts/test_railway_deploy_script.py @@ -41,7 +41,7 @@ cat <> "$FAKE_CURL_LOG" +printf '{"build": {"commit_sha": "%s"}}' "$FAKE_CURL_SHA" > "$out" +printf '200' +""" + def _git(cwd: Path, *args: str) -> str: return subprocess.run( @@ -313,20 +332,48 @@ def test_requires_backend_url_when_the_environment_is_chosen_by_hand(harness: _H assert harness.stamp_file() == _PLACEHOLDER -def test_refuses_a_non_production_linked_environment_without_backend_url(harness: _Harness) -> None: +@pytest.mark.parametrize( + ("variable", "value", "expected"), + [ + ("FAKE_RAILWAY_ENV_NAME", "staging", "environment 'staging'"), + ("FAKE_RAILWAY_PROJECT_NAME", "Other-Project", "project 'Other-Project'"), + ("FAKE_RAILWAY_SERVICE_NAME", "Worker", "service 'Worker'"), + ], +) +def test_refuses_the_default_url_for_any_other_linked_target( + harness: _Harness, variable: str, value: str, expected: str +) -> None: + # The production default URL belongs to one project/environment/service. + # A checkout linked anywhere else must say where to verify. harness.health.sha = harness.head() + result = harness.run(RAILWAY_SERVICE_ID=None, RAILWAY_ENVIRONMENT_ID=None, BACKEND_URL=None, **{variable: value}) + + assert result.returncode != 0 + assert expected in result.stderr + assert "default BACKEND_URL belongs to RegEngine-Prod / production / RegEngine" in result.stderr + assert harness.calls() == [["status"]] + assert harness.stamp_file() == _PLACEHOLDER + + +def test_uses_the_default_url_for_the_linked_production_target(harness: _Harness) -> None: + # No network: a fake curl answers for whatever URL the script chooses. + fake_curl = harness.bin_dir / "curl" + fake_curl.write_text(_FAKE_CURL, encoding="utf-8") + fake_curl.chmod(0o755) + curl_log = harness.log_dir / "curl.log" + result = harness.run( RAILWAY_SERVICE_ID=None, RAILWAY_ENVIRONMENT_ID=None, BACKEND_URL=None, - FAKE_RAILWAY_ENV_NAME="staging", + FAKE_CURL_LOG=str(curl_log), + FAKE_CURL_SHA=harness.head(), ) - assert result.returncode != 0 - assert "'staging', not production" in result.stderr - assert harness.calls() == [["status"]] - assert harness.stamp_file() == _PLACEHOLDER + assert result.returncode == 0, result.stderr + assert [call[0] for call in harness.calls()] == ["status", "up"] + assert curl_log.read_text(encoding="utf-8").splitlines() == ["https://regengine-production.up.railway.app/health"] def test_fails_when_health_never_reports_the_shipped_sha(harness: _Harness) -> None: From db97abc127050ae0fb393436ccbdae24c978f5db Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Fri, 30 Oct 2026 21:55:49 -0700 Subject: [PATCH 4/6] ci(deploy): require https for BACKEND_URL except loopback The /health body is what the script trusts for the shipped SHA, so a plain-http public URL would let an on-path party forge a green result. Accept https:// URLs, and http:// only for 127.0.0.1, localhost and [::1], which is what the tests use. Checked before anything is touched. Co-Authored-By: Claude Fable 5.1 --- AGENTS.md | 2 +- scripts/railway_deploy.sh | 16 +++++++++++++++- tests/scripts/test_railway_deploy_script.py | 13 +++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bdcd587d5..81450c420 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,7 +110,7 @@ scripts/railway_deploy.sh - It stamps the HEAD SHA into `build-sha.txt` for the upload only and restores the tracked placeholder afterwards. Never commit a real SHA into that file. - It runs `railway up --ci` with the service and environment from `RAILWAY_SERVICE_ID` / `RAILWAY_ENVIRONMENT_ID`, or from the linked project in `railway status`, plus `--project` when `RAILWAY_PROJECT_ID` is set, and attaches the message ` @`. - Run it from the primary checkout, not a git worktree: `railway up` cannot index a checkout whose `.git` is a file, and the script refuses one. -- The default health URL belongs to one target: project RegEngine-Prod, environment production, service RegEngine, as `railway status` reports them. For anything else (an id picked by hand, or another linked project, environment or service) set `BACKEND_URL` to that target's public URL; the script refuses otherwise, because the production default could pass while the real target never served the upload. +- The default health URL belongs to one target: project RegEngine-Prod, environment production, service RegEngine, as `railway status` reports them. For anything else (an id picked by hand, or another linked project, environment or service) set `BACKEND_URL` to that target's public https URL; the script refuses otherwise, because the production default could pass while the real target never served the upload. - It then polls `${BACKEND_URL:-https://regengine-production.up.railway.app}/health` for up to 900 s and exits non-zero unless `.build.commit_sha` equals the SHA it shipped. Quote that line, not the upload, when reporting that a change is live. - Never run a bare `railway up`: the image it ships cannot say which commit it is, and `/health` answers `build.commit_sha: null`. The older `scripts/deploy_railway.sh` pushes `main` and waits for a GitHub-triggered deploy that no longer exists; do not use it. - Deploying is a production action. Do it only when Christopher asks for that deploy. diff --git a/scripts/railway_deploy.sh b/scripts/railway_deploy.sh index 6c4aa61b7..0d71c5867 100755 --- a/scripts/railway_deploy.sh +++ b/scripts/railway_deploy.sh @@ -39,7 +39,9 @@ # service RegEngine) and is used only when `railway status` reports that # exact linked target. Any id picked by hand, or any other linked target, # must set BACKEND_URL explicitly, or the check could pass against -# production while the real target never served the upload. +# production while the real target never served the upload. It must be +# an https:// URL; plain http:// is accepted for loopback only, because +# the /health body is what this script trusts for the shipped SHA. # RAILWAY_DEPLOY_TIMEOUT_SECONDS # Rollout budget for the /health check. Default 900, matching deploy.yml. # RAILWAY_DEPLOY_POLL_SECONDS @@ -98,6 +100,18 @@ DEFAULT_TARGET_SERVICE="RegEngine" BACKEND_URL_SET=1 [ -n "${BACKEND_URL:-}" ] || BACKEND_URL_SET=0 BACKEND_URL="${BACKEND_URL:-$DEFAULT_BACKEND_URL}" +# The /health body is trusted for the shipped SHA, so it has to arrive over +# TLS. Plain http is allowed only to loopback (what the tests use). +case "$BACKEND_URL" in + https://*) ;; + http://127.0.0.1 | http://127.0.0.1:* | http://127.0.0.1/* | \ + http://localhost | http://localhost:* | http://localhost/* | \ + "http://[::1]" | "http://[::1]:"* | "http://[::1]/"*) ;; + *) + echo "error: BACKEND_URL must be an https:// URL (plain http:// is accepted for loopback only)." >&2 + exit 1 + ;; +esac TIMEOUT_SECONDS="${RAILWAY_DEPLOY_TIMEOUT_SECONDS:-900}" POLL_SECONDS="${RAILWAY_DEPLOY_POLL_SECONDS:-15}" diff --git a/tests/scripts/test_railway_deploy_script.py b/tests/scripts/test_railway_deploy_script.py index c2f2fe4e3..3b56c35ac 100644 --- a/tests/scripts/test_railway_deploy_script.py +++ b/tests/scripts/test_railway_deploy_script.py @@ -319,6 +319,19 @@ def test_reads_service_and_environment_from_railway_status_when_unset(harness: _ assert "svc-from-status" not in result.stdout + result.stderr +def test_refuses_a_plain_http_backend_url_that_is_not_loopback(harness: _Harness) -> None: + # The /health body is trusted for the shipped SHA, so a public URL must be + # https; the loopback http URL the other tests use stays allowed. + harness.health.sha = harness.head() + + result = harness.run(BACKEND_URL="http://health.example.invalid:8080") + + assert result.returncode != 0 + assert "BACKEND_URL must be an https:// URL" in result.stderr + assert harness.calls() == [] + assert harness.stamp_file() == _PLACEHOLDER + + def test_requires_backend_url_when_the_environment_is_chosen_by_hand(harness: _Harness) -> None: # RAILWAY_ENVIRONMENT_ID may point at staging; the production default URL # would then verify the wrong deployment and could pass by accident. From f9bb1dca63ad0edee20fdb85b57b6c1451cf8719 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Fri, 30 Oct 2026 22:03:24 -0700 Subject: [PATCH 5/6] ci(deploy): close the userinfo bypass in the loopback http allowance `http://127.0.0.1:*` as a shell glob also matched `http://127.0.0.1:80@attacker.example`, where the loopback text is user information and the real host is the attacker's. Match loopback with an anchored regex instead: the host may be followed only by a decimal port and then the end of the URL or a path. Regression tests cover the userinfo form for 127.0.0.1, localhost and [::1]. Co-Authored-By: Claude Fable 5.1 --- scripts/railway_deploy.sh | 21 ++++++++++----------- tests/scripts/test_railway_deploy_script.py | 14 ++++++++++++-- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/scripts/railway_deploy.sh b/scripts/railway_deploy.sh index 0d71c5867..46d49973d 100755 --- a/scripts/railway_deploy.sh +++ b/scripts/railway_deploy.sh @@ -101,17 +101,16 @@ BACKEND_URL_SET=1 [ -n "${BACKEND_URL:-}" ] || BACKEND_URL_SET=0 BACKEND_URL="${BACKEND_URL:-$DEFAULT_BACKEND_URL}" # The /health body is trusted for the shipped SHA, so it has to arrive over -# TLS. Plain http is allowed only to loopback (what the tests use). -case "$BACKEND_URL" in - https://*) ;; - http://127.0.0.1 | http://127.0.0.1:* | http://127.0.0.1/* | \ - http://localhost | http://localhost:* | http://localhost/* | \ - "http://[::1]" | "http://[::1]:"* | "http://[::1]/"*) ;; - *) - echo "error: BACKEND_URL must be an https:// URL (plain http:// is accepted for loopback only)." >&2 - exit 1 - ;; -esac +# TLS. Plain http is allowed only to loopback (what the tests use). The +# loopback host may be followed only by a decimal port and then the end of +# the URL or a path: a glob such as `http://127.0.0.1:*` would also accept +# `http://127.0.0.1:80@attacker.example`, where the loopback text is user +# information and the real host is the attacker's. +LOOPBACK_HTTP_RE='^http://(127\.0\.0\.1|localhost|\[::1\])(:[0-9]+)?(/.*)?$' +if [[ "$BACKEND_URL" != https://* ]] && [[ ! "$BACKEND_URL" =~ $LOOPBACK_HTTP_RE ]]; then + echo "error: BACKEND_URL must be an https:// URL (plain http:// is accepted for loopback only)." >&2 + exit 1 +fi TIMEOUT_SECONDS="${RAILWAY_DEPLOY_TIMEOUT_SECONDS:-900}" POLL_SECONDS="${RAILWAY_DEPLOY_POLL_SECONDS:-15}" diff --git a/tests/scripts/test_railway_deploy_script.py b/tests/scripts/test_railway_deploy_script.py index 3b56c35ac..5bf8a4ec3 100644 --- a/tests/scripts/test_railway_deploy_script.py +++ b/tests/scripts/test_railway_deploy_script.py @@ -319,12 +319,22 @@ def test_reads_service_and_environment_from_railway_status_when_unset(harness: _ assert "svc-from-status" not in result.stdout + result.stderr -def test_refuses_a_plain_http_backend_url_that_is_not_loopback(harness: _Harness) -> None: +@pytest.mark.parametrize( + "url", + [ + "http://health.example.invalid:8080", + # Loopback text as user information; the real host is the attacker's. + "http://127.0.0.1:80@health.example.invalid", + "http://localhost@health.example.invalid", + "http://[::1]:80@health.example.invalid", + ], +) +def test_refuses_a_plain_http_backend_url_that_is_not_loopback(harness: _Harness, url: str) -> None: # The /health body is trusted for the shipped SHA, so a public URL must be # https; the loopback http URL the other tests use stays allowed. harness.health.sha = harness.head() - result = harness.run(BACKEND_URL="http://health.example.invalid:8080") + result = harness.run(BACKEND_URL=url) assert result.returncode != 0 assert "BACKEND_URL must be an https:// URL" in result.stderr From a16ff6f8ea673afaf4ce4b4942da1e1f246b9e8a Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Fri, 30 Oct 2026 22:06:06 -0700 Subject: [PATCH 6/6] ci(deploy): bypass proxies for loopback health probes http_proxy or ALL_PROXY would route even a loopback /health request through a proxy that could forge the body the script trusts. Pass `--noproxy '*'` when BACKEND_URL is loopback. The https case is left alone: TLS protects it end to end, so a corporate proxy stays usable. A test sets the proxy variables to a closed port and expects success. Co-Authored-By: Claude Fable 5.1 --- scripts/railway_deploy.sh | 9 ++++++++- tests/scripts/test_railway_deploy_script.py | 18 +++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/scripts/railway_deploy.sh b/scripts/railway_deploy.sh index 46d49973d..0920d8f99 100755 --- a/scripts/railway_deploy.sh +++ b/scripts/railway_deploy.sh @@ -111,6 +111,13 @@ if [[ "$BACKEND_URL" != https://* ]] && [[ ! "$BACKEND_URL" =~ $LOOPBACK_HTTP_RE echo "error: BACKEND_URL must be an https:// URL (plain http:// is accepted for loopback only)." >&2 exit 1 fi +# A proxy named by http_proxy or ALL_PROXY must not sit between this script +# and a loopback health endpoint, or it could forge the body. TLS protects the +# https case end to end, so a corporate proxy stays usable there. +CURL_PROXY_ARGS=() +if [[ "$BACKEND_URL" =~ $LOOPBACK_HTTP_RE ]]; then + CURL_PROXY_ARGS=(--noproxy '*') +fi TIMEOUT_SECONDS="${RAILWAY_DEPLOY_TIMEOUT_SECONDS:-900}" POLL_SECONDS="${RAILWAY_DEPLOY_POLL_SECONDS:-15}" @@ -251,7 +258,7 @@ echo "waiting up to ${TIMEOUT_SECONDS}s for ${BACKEND_URL}/health to report ${EX while [ "$SECONDS" -lt "$deadline" ]; do attempt=$(( attempt + 1 )) body=$(mktemp) - last_code=$(curl -sS --max-time 20 -o "$body" -w '%{http_code}' "${BACKEND_URL}/health" || true) + last_code=$(curl -sS --max-time 20 ${CURL_PROXY_ARGS[@]+"${CURL_PROXY_ARGS[@]}"} -o "$body" -w '%{http_code}' "${BACKEND_URL}/health" || true) last_code="${last_code:-000}" if [ "$last_code" = "200" ]; then diff --git a/tests/scripts/test_railway_deploy_script.py b/tests/scripts/test_railway_deploy_script.py index 5bf8a4ec3..b14ab0020 100644 --- a/tests/scripts/test_railway_deploy_script.py +++ b/tests/scripts/test_railway_deploy_script.py @@ -70,7 +70,7 @@ url="" while [ $# -gt 0 ]; do case "$1" in - -o|-w|--max-time) out_or_opt="$1"; [ "$out_or_opt" = "-o" ] && out="$2"; shift 2 ;; + -o|-w|--max-time|--noproxy) out_or_opt="$1"; [ "$out_or_opt" = "-o" ] && out="$2"; shift 2 ;; -*) shift ;; *) url="$1"; shift ;; esac @@ -306,6 +306,22 @@ def test_stamps_head_sha_for_the_upload_and_restores_the_placeholder(harness: _H assert "proj-from-env" not in output +def test_loopback_health_check_ignores_proxy_variables(harness: _Harness) -> None: + # A proxy from the environment must not sit between the script and the + # loopback /health it trusts. Port 1 is closed, so a proxied request would + # fail with status 000 and the run would time out instead of passing. + harness.health.sha = harness.head() + + result = harness.run( + http_proxy="http://127.0.0.1:1", + HTTP_PROXY="http://127.0.0.1:1", + ALL_PROXY="http://127.0.0.1:1", + ) + + assert result.returncode == 0, result.stderr + assert f"Deployed commit matches: {harness.head()}" in result.stdout + + def test_reads_service_and_environment_from_railway_status_when_unset(harness: _Harness) -> None: harness.health.sha = harness.head()