From 90d0613330731a7fca7142c226a623243f662541 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:15:04 +0300 Subject: [PATCH 01/10] Check nightly that a new user can still reach a first cloud build The onboarding starter is generated and personalised server-side, then resolves its dependencies over the network on the user's own machine. Neither half is visible to a check that reads the template out of a repository or replaces Maven with a stub, and both have broken in ways that made a first build impossible while every offline check stayed green: a generated pom that declared no dependency repository, so a pinned release resolved nothing once Codename One moved off Maven Central; launchers shipped without the execute bit, so the first command the README documents failed with "permission denied"; and a batch launcher that fell through to producing a local jar when given no target, so the user got no cloud build and no error either. Each survived for weeks because nothing exercised the artefact a user actually receives. So this signs in, downloads the personalised starter from the console, runs the shipped launcher with a real Maven, and requires the cloud build to reach success, on Linux and Windows, nightly. On failure it opens a single issue, assigns it, re-comments at most daily, and closes it on recovery -- the same pattern the syndication watchdog uses. Two details worth keeping. Success is read from the console's own build list rather than launcher stdout, because the upload client ships as a binary dependency whose wording is not in this repository and could change without notice. And a build is identified by an id that was absent before launch, since an account may retain only its most recent build, so counting rows would not do -- which is also what detects the local-jar case precisely: the launcher exits clean and no build ever appears. Apple targets are refused outright; they cost several times a normal build and a nightly run would exhaust the account. The assertions have offline tests, run on any PR that touches them, each pinned to the breakage it was written for. A canary that decays into a green check verifying nothing is the failure mode this is meant to end. Until the three secrets exist the scheduled run fails and opens the tracking issue, which is correct: an unconfigured canary is not a passing one. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/starter-canary.yml | 231 ++++++++++ scripts/ci/starter-canary/README.md | 67 +++ scripts/ci/starter-canary/starter_canary.py | 434 ++++++++++++++++++ .../ci/starter-canary/test_starter_canary.py | 161 +++++++ 4 files changed, 893 insertions(+) create mode 100644 .github/workflows/starter-canary.yml create mode 100644 scripts/ci/starter-canary/README.md create mode 100755 scripts/ci/starter-canary/starter_canary.py create mode 100644 scripts/ci/starter-canary/test_starter_canary.py diff --git a/.github/workflows/starter-canary.yml b/.github/workflows/starter-canary.yml new file mode 100644 index 00000000000..70de9f61426 --- /dev/null +++ b/.github/workflows/starter-canary.yml @@ -0,0 +1,231 @@ +name: Cloud starter canary + +# Exercises the onboarding starter a NEW USER actually receives, end to end: +# sign in, download the personalised ZIP from the console, run the shipped +# launcher with a real Maven, and require the cloud build to finish. +# +# The starter is generated and personalised server-side and then resolves its +# dependencies over the network on the user's machine. Neither is visible to a +# check that reads the template out of a repository or stubs Maven out, and both +# have broken in ways that made a first build impossible while every offline +# check stayed green. So this runs against the live service, nightly. +# +on: + # Keep the assertions honest on a PR that edits them. This leg runs no build + # and touches no network, so it costs seconds. + pull_request: + paths: + - 'scripts/ci/starter-canary/**' + - '.github/workflows/starter-canary.yml' + schedule: + # 04:40 UTC: after the server-side weekly version refresh has settled, and + # away from the blog/syndication slots. + - cron: '40 4 * * *' + workflow_dispatch: + inputs: + target: + description: 'Build target (never iphone/macos -- 8 credits each)' + default: 'javascript' + skip_build: + description: 'Artefact checks only, no cloud build' + type: boolean + default: false + +permissions: + contents: read + issues: write + +concurrency: + # Never let two canaries submit builds at once: a free/basic account is + # limited to one in-flight build, so the second would fail for a reason that + # has nothing to do with the starter. + group: starter-canary + cancel-in-progress: false + +jobs: + self-test: + name: Canary assertions still fire + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-python@v6 + with: + python-version: '3.12' + - run: python3 scripts/ci/starter-canary/test_starter_canary.py + + canary: + # Never talk to production from a pull request: forks get no secrets, and a + # PR must not spend the canary account's build credits. + if: github.event_name != 'pull_request' + strategy: + # One runner must not mask the other: past breakages have been + # unix-only (the execute bit) and Windows-only (the batch launcher). + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} + timeout-minutes: 45 + steps: + - uses: actions/checkout@v6 + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: '3.12' + + # The starter enforces JDK 17+ through maven-enforcer; give the runner a + # JDK that satisfies the same gate a user's machine has to. + - name: Set up JDK 17 + uses: actions/setup-java@v5 + with: + java-version: '17' + distribution: 'temurin' + + - name: Run the canary + id: canary + continue-on-error: true + env: + CN1_CANARY_EMAIL: ${{ secrets.CN1_CANARY_EMAIL }} + CN1_CANARY_PASSWORD: ${{ secrets.CN1_CANARY_PASSWORD }} + CN1_CANARY_TOKEN: ${{ secrets.CN1_CANARY_TOKEN }} + CANARY_REPORT: ${{ runner.temp }}/canary.json + run: | + python3 scripts/ci/starter-canary/starter_canary.py \ + --target "${{ github.event.inputs.target || 'javascript' }}" \ + ${{ github.event.inputs.skip_build == 'true' && '--skip-build' || '' }} \ + --report "${{ runner.temp }}/canary.json" + + - name: Ensure a report exists + if: always() + shell: bash + run: | + report="${{ runner.temp }}/canary.json" + if [ ! -f "$report" ]; then + printf '%s' '{"ok":false,"error":"the canary produced no report at all -- it crashed before it could write one."}' > "$report" + fi + cat "$report" + + - name: Upload report + if: always() + uses: actions/upload-artifact@v4 + with: + name: canary-${{ matrix.os }} + path: ${{ runner.temp }}/canary.json + + - name: Fail the job when the canary failed + if: steps.canary.outcome != 'success' + run: exit 1 + + alert: + needs: canary + if: always() && github.event_name != 'pull_request' + runs-on: ubuntu-latest + permissions: + contents: read + issues: write + steps: + - name: Collect the per-runner reports + uses: actions/download-artifact@v4 + with: + path: reports + pattern: canary-* + continue-on-error: true + + - name: Open, update, or close the starter alert + uses: actions/github-script@v8 + env: + # Assign directly so this is harder to miss than an Actions-only + # failure, mirroring the syndication watchdog. + ALERT_ASSIGNEE: ${{ vars.STARTER_CANARY_ASSIGNEE || 'shai-almog' }} + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const title = 'Cloud starter is broken for new users'; + const marker = ''; + const reminderMarker = ''; + + const fs = require('fs'); + const read = (dir, label) => { + const file = `reports/canary-${dir}/canary.json`; + if (!fs.existsSync(file)) { + return `${label}: the canary job produced no report (it did not run, or the runner died).`; + } + const raw = fs.readFileSync(file, 'utf8'); + try { + const data = JSON.parse(raw); + return data.ok ? null : `${label}: ${data.error}`; + } catch (e) { + return `${label}: unreadable canary report -- ${raw.slice(0, 300)}`; + } + }; + + const problems = [ + read('ubuntu-latest', 'Linux'), + read('windows-latest', 'Windows'), + ].filter(Boolean); + + const issues = await github.paginate(github.rest.issues.listForRepo, { + owner, repo, state: 'open', per_page: 100, + }); + let issue = issues.find(candidate => + !candidate.pull_request && + candidate.title === title && + (candidate.body || '').includes(marker) + ); + + if (problems.length === 0) { + await core.summary.addHeading('Cloud starter canary').addRaw('Healthy').write(); + if (issue) { + await github.rest.issues.createComment({ + owner, repo, issue_number: issue.number, + body: `${reminderMarker}\nRecovered at ${new Date().toISOString()}. A new user can download the starter and complete a cloud build on both Linux and Windows.`, + }); + await github.rest.issues.update({ + owner, repo, issue_number: issue.number, + state: 'closed', state_reason: 'completed', + }); + } + return; + } + + const run = `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`; + const body = [ + marker, + 'The nightly canary could not complete a first cloud build the way a new user does.', + '', + ...problems.map(p => `- ${p}`), + '', + `Run: ${run}`, + `Last checked: ${new Date().toISOString()}`, + '', + 'This check signs in, downloads the personalised starter from the console, and runs the shipped launcher against the live service. A failure here means a new user cannot complete a first cloud build at all. The cause is usually server-side: the starter generator, the vendored starter template, or the console sign-in redirect.', + ].join('\n'); + + const assignees = process.env.ALERT_ASSIGNEE ? [process.env.ALERT_ASSIGNEE] : undefined; + if (!issue) { + const created = await github.rest.issues.create({ owner, repo, title, body, assignees }); + issue = created.data; + } else { + await github.rest.issues.update({ owner, repo, issue_number: issue.number, body, assignees }); + const comments = await github.paginate(github.rest.issues.listComments, { + owner, repo, issue_number: issue.number, per_page: 100, + }); + const reminders = comments.filter(c => (c.body || '').includes(reminderMarker)); + const last = reminders.at(-1); + const ageHours = last ? (Date.now() - Date.parse(last.created_at)) / 3600000 : Infinity; + if (ageHours >= 24) { + await github.rest.issues.createComment({ + owner, repo, issue_number: issue.number, + body: `${reminderMarker}\nStill failing as of ${new Date().toISOString()}. See the updated issue body.`, + }); + } + } + + await core.summary + .addHeading('Cloud starter canary failure') + .addList(problems) + .addLink(`Tracking issue #${issue.number}`, issue.html_url) + .write(); + core.setFailed(`The cloud starter is broken; tracking issue #${issue.number}`); diff --git a/scripts/ci/starter-canary/README.md b/scripts/ci/starter-canary/README.md new file mode 100644 index 00000000000..ae099e56770 --- /dev/null +++ b/scripts/ci/starter-canary/README.md @@ -0,0 +1,67 @@ +# Cloud starter canary + +Nightly end-to-end check that a new user can still reach a first cloud build: +sign in, download the personalised starter from the console, run the shipped +launcher with a real Maven, and require the cloud build to finish — on Linux +and Windows. + +## Why it runs against the live service + +The starter is generated and personalised server-side, then resolves its +dependencies over the network on the user's own machine. Neither of those is +visible to a check that reads the template out of a repository or replaces +Maven with a stub — and both have broken in ways that made a first build +impossible while every offline check stayed green: a generated pom with no +dependency repository, launchers shipped without the execute bit, and a batch +launcher that quietly produced a local jar instead of submitting a build. + +## Setup + +1. Create a dedicated build account for the canary. +2. Add three repository secrets: + + | Secret | Value | + |---|---| + | `CN1_CANARY_EMAIL` | the account's email | + | `CN1_CANARY_PASSWORD` | its console password, for the starter download | + | `CN1_CANARY_TOKEN` | its build token, for headless build-client auth | + + Optionally set the `STARTER_CANARY_ASSIGNEE` repository *variable* to change + who gets assigned the alert issue. +3. Run it once by hand — **Actions → Cloud starter canary → Run workflow** — + before trusting the schedule. + +Until the secrets exist the scheduled run fails and opens the tracking issue. +An unconfigured canary is not a passing one. + +## Running it locally + +```bash +export CN1_CANARY_EMAIL=... CN1_CANARY_PASSWORD=... CN1_CANARY_TOKEN=... + +# artefact checks only, no build submitted +python3 scripts/ci/starter-canary/starter_canary.py --skip-build + +# the full journey +python3 scripts/ci/starter-canary/starter_canary.py --target javascript +``` + +Apple targets cost several times a normal build and the script refuses them. + +## The assertions + +`test_starter_canary.py` runs on every PR touching this directory and proves +each assertion still fires for the breakage it was written for. A canary that +decays into a green check verifying nothing is the failure mode it exists to +end. + +```bash +python3 scripts/ci/starter-canary/test_starter_canary.py +``` + +## When it fails + +The alert job opens or updates a single issue, assigns it, and re-comments at +most once a day while it stays broken, closing it automatically on recovery. +The cause is usually server-side: the starter generator, the vendored starter +template, or the console sign-in redirect. diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py new file mode 100755 index 00000000000..4709eed8426 --- /dev/null +++ b/scripts/ci/starter-canary/starter_canary.py @@ -0,0 +1,434 @@ +#!/usr/bin/env python3 +"""Black-box canary for the Codename One cloud onboarding starter. + +Does exactly what a new user does, against the live build service: + + 1. sign in and pull the personalised starter ZIP from the console (the same + session-cookie endpoint the onboarding checklist links to) + 2. unzip it into a path containing a space and an apostrophe + 3. assert the properties that have silently broken before -- launcher execute + bits, and a / pair that can actually + resolve Codename One + 4. run the SHIPPED launcher (build.sh / build.bat) with a real Maven and + require the cloud build to reach a terminal success + +Why a live end-to-end check and not a unit test: the starter a user receives is +generated and personalised server-side, then resolves its dependencies over the +network on the user's own machine. Neither of those is visible to a test that +reads the template out of a repository or replaces Maven with a stub, and both +have broken in ways that made the first build impossible while every offline +check stayed green: + + * the generated pom declared no dependency repository at all, so once + Codename One releases moved off Maven Central a pinned starter could not + resolve anything + * the ZIP shipped build.sh/run.sh/mvnw without the execute bit, so the first + command the README documents failed with "permission denied" + * build.bat fell through to producing a local jar when given no target, so + the user got no cloud build and no error either + +Each survived for weeks because nothing exercised the artefact a user actually +receives. This runs nightly and opens an issue when it breaks. +""" +import argparse +import json +import os +import re +import shutil +import stat +import subprocess +import sys +import tempfile +import time +import urllib.error +import urllib.parse +import urllib.request +import zipfile +from http.cookiejar import CookieJar +from pathlib import Path + +WINDOWS = os.name == "nt" +# A directory whose name breaks naive quoting. A shipped mvnw.cmd once +# interpolated the path into PowerShell source, so a space or an apostrophe was +# a build failure; keep reproducing that shape here. +AWKWARD = "Project O'Brien with spaces" + + +class CanaryFailure(RuntimeError): + """A user-visible breakage. The message becomes the GitHub issue body.""" + + +REPORT_PATH = None + + +def log(message): + print(f"[canary] {message}", flush=True) + + +def build_opener(): + jar = CookieJar() + return urllib.request.build_opener( + urllib.request.HTTPCookieProcessor(jar), + urllib.request.HTTPRedirectHandler(), + ), jar + + +def fetch(opener, url, data=None, headers=None): + request = urllib.request.Request(url, data=data, headers=headers or {}) + request.add_header("User-Agent", "cn1-starter-canary") + try: + with opener.open(request, timeout=120) as response: + return response.status, response.read(), response.headers, response.url + except urllib.error.HTTPError as error: + return error.code, error.read(), error.headers, url + + +def login(opener, base, email, password): + """Sign in through the ordinary HTML form, exactly as a person does.""" + status, body, _, _ = fetch(opener, f"{base}/login") + if status != 200: + raise CanaryFailure(f"GET /login returned HTTP {status}; production may be down") + token = extract_csrf(body.decode("utf-8", "replace")) + form = {"username": email, "password": password} + if token: + form[token[0]] = token[1] + status, body, _, final = fetch( + opener, + f"{base}/login", + data=urllib.parse.urlencode(form).encode(), + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + if status not in (200, 302) or "error" in urllib.parse.urlparse(final).query: + raise CanaryFailure( + f"form login as {email} failed (HTTP {status}, landed on {final}). " + "Either the canary credentials are wrong or the sign-in path is broken." + ) + return final + + +def extract_csrf(html): + match = re.search( + r']+name="(_csrf[^"]*)"[^>]+value="([^"]*)"', html + ) or re.search( + r']+value="([^"]*)"[^>]+name="(_csrf[^"]*)"', html + ) + if not match: + return None + a, b = match.group(1), match.group(2) + return (a, b) if a.startswith("_csrf") else (b, a) + + +def download_starter(opener, base, target, destination): + url = f"{base}/api/v2/console/onboarding/starter.zip?source=canary" + if target: + url += f"&target={urllib.parse.quote(target)}" + status, body, headers, _ = fetch(opener, url) + if status == 403: + raise CanaryFailure( + "starter.zip returned 403 -- the session did not carry through login. " + "A signed-in user cannot download the starter." + ) + if status != 200: + raise CanaryFailure(f"starter.zip returned HTTP {status}") + kind = (headers.get("Content-Type") or "").lower() + if body[:2] != b"PK": + raise CanaryFailure( + f"starter.zip was not a ZIP (Content-Type {kind!r}, {len(body)} bytes). " + "The generator is serving something else -- most likely an error page." + ) + destination.write_bytes(body) + log(f"downloaded starter.zip ({len(body)} bytes)") + return destination + + +def unpack(archive, into): + """Unzip preserving the unix mode, which is the whole point of the check.""" + into.mkdir(parents=True, exist_ok=True) + modes = {} + try: + with zipfile.ZipFile(archive) as zf: + zf.extractall(into) + for info in zf.infolist(): + mode = info.external_attr >> 16 + if mode: + modes[info.filename] = mode + except (zipfile.BadZipFile, OSError) as error: + raise CanaryFailure( + f"the served starter could not be unpacked: {error}. " + "A user who clicks Download gets a file that will not open." + ) from error + for name, mode in modes.items(): + path = into / name + if path.exists() and not path.is_dir(): + path.chmod(mode & 0o7777) + roots = [p for p in into.iterdir() if p.is_dir()] + if len(roots) != 1: + raise CanaryFailure( + f"expected exactly one directory inside starter.zip, found {[p.name for p in roots]}" + ) + return roots[0], modes + + +def check_launcher_bits(project, modes): + """The launchers must be executable in the ZIP the server hands out.""" + if WINDOWS: + # Windows has no unix mode; read what the ZIP recorded instead, which is + # what a macOS or Linux user would actually get. + broken = [ + name for name, mode in modes.items() + if Path(name).name in ("build.sh", "run.sh", "mvnw") and not (mode & 0o111) + ] + else: + broken = [] + for name in ("build.sh", "run.sh", "mvnw"): + path = project / name + if path.exists() and not (path.stat().st_mode & stat.S_IXUSR): + broken.append(name) + if broken: + raise CanaryFailure( + f"the served starter ZIP has non-executable launchers: {', '.join(sorted(broken))}. " + "Every macOS/Linux user gets 'permission denied' on the first documented command. " + "Check that the server-side generator sets a unix mode on the launchers." + ) + log("launcher execute bits OK") + + +def check_repositories(project): + """A pinned release with no repository declared resolves nothing.""" + pom = (project / "pom.xml").read_text(encoding="utf-8", errors="replace") + version = None + match = re.search(r"\s*([^<\s]+)\s*", pom) \ + or re.search(r"\s*([^<\s]+)\s*", pom) + if match: + version = match.group(1) + missing = [ + block for block in ("repositories", "pluginRepositories") + if f"<{block}>" not in pom + ] + if missing: + raise CanaryFailure( + f"the served starter pom.xml declares no <{'> and no <'.join(missing)}>. " + "Codename One left Maven Central at 7.0.268, so a pin past 7.0.267 resolves " + "nothing on a user's machine." + ) + if "repo.codenameone.com" not in pom: + raise CanaryFailure( + "the served starter pom.xml declares repositories but none pointing at " + "repo.codenameone.com -- releases past 7.0.267 live only there." + ) + if version and version <= "7.0.267": + log(f"WARNING: starter pins cn1 {version}, at or below the Maven Central freeze point") + log(f"repository declarations OK (pinned version: {version or 'unknown'})") + return version + + +def seed_token(project, mvn, email, token, version): + """Headless build-client auth, so no browser OAuth is needed in CI. + + The goal writes into the java Preferences node the build client reads, so it + needs no project state -- but it does need an explicit plugin version. + An unversioned groupId:artifactId:goal makes Maven resolve LATEST, which is + precisely the sort of implicit resolution this canary exists to catch. + """ + if not version: + raise CanaryFailure( + "could not read the Codename One version out of the served starter pom, " + "so the build client cannot be authenticated with a pinned plugin." + ) + run( + [mvn, "-B", "-q", + f"com.codenameone:codenameone-maven-plugin:{version}:set-user-token", + f"-Dtoken={token}", f"-Duser={email}"], + cwd=project, + what="cn1:set-user-token", + secret=token, + ) + log("seeded build-client token") + + +def run(command, cwd, what, timeout=3600, secret=None, check=True): + result = subprocess.run( + command, cwd=str(cwd), capture_output=True, text=True, timeout=timeout + ) + output = (result.stdout or "") + (result.stderr or "") + if secret: + output = output.replace(secret, "***") + if check and result.returncode != 0: + raise CanaryFailure( + f"{what} failed with exit {result.returncode}:\n{tail(output)}" + ) + return result.returncode, output + + +def tail(text, lines=40): + rows = [r for r in text.splitlines() if r.strip()] + return "\n".join(rows[-lines:]) + + +def launch(project, target): + """Run the launcher the README tells the user to run -- not mvn directly.""" + if WINDOWS: + command = ["cmd", "/c", "build.bat", target] + else: + launcher = project / "build.sh" + if not os.access(launcher, os.X_OK): + raise CanaryFailure("build.sh is present but not executable") + command = ["./build.sh", target] + env_note = f"{'build.bat' if WINDOWS else './build.sh'} {target}" + log(f"running {env_note} (this downloads Maven and the CN1 toolchain; several minutes)") + code, output = run( + command, cwd=project, what=env_note, timeout=3600, check=False + ) + return code, output + + +def list_builds(opener, base): + """Authoritative build state, straight from the console API. + + Matching launcher stdout for phrases like "sent to the build server" would + be guesswork -- the upload client ships as a binary dependency, so its exact + wording is not in this repository and could change without notice. The + console's own build list is what the user sees in the web UI, so assert + against that instead. + """ + status, body, _, _ = fetch(opener, f"{base}/api/v2/console/builds") + if status != 200: + raise CanaryFailure(f"GET /api/v2/console/builds returned HTTP {status}") + try: + return json.loads(body.decode("utf-8", "replace")).get("builds", []) + except ValueError as error: + raise CanaryFailure(f"the console build list was not JSON: {error}") from error + + +TERMINAL = {"success", "failed", "cancelled"} + + +def await_cloud_build(opener, base, known_ids, target, launcher_code, output, + timeout=1800, interval=20): + """Wait for a build this run submitted to reach a terminal state.""" + deadline = time.time() + timeout + seen = None + while time.time() < deadline: + fresh = [b for b in list_builds(opener, base) if b.get("id") not in known_ids] + if fresh: + fresh.sort(key=lambda b: b.get("submittedAt") or 0) + seen = fresh[-1] + if (seen.get("status") or "").lower() in TERMINAL: + break + time.sleep(interval) + + if seen is None: + # The launcher "succeeded" locally and nothing was ever submitted, so + # the user is left holding a jar and no cloud build. + raise CanaryFailure( + f"running the documented launcher for '{target}' never created a build on the " + f"server (launcher exit {launcher_code}). A new user following the README gets " + f"no cloud build.\n\n{tail(output, 60)}" + ) + + state = (seen.get("status") or "unknown").lower() + if state == "success": + log(f"cloud build {seen.get('id')} for '{target}' finished: success") + return seen + if state in TERMINAL: + raise CanaryFailure( + f"the cloud build for '{target}' finished as '{state}' " + f"(build {seen.get('id')}): {seen.get('message') or 'no message'}" + ) + raise CanaryFailure( + f"the cloud build for '{target}' (build {seen.get('id')}) was still '{state}' " + f"after {timeout}s" + ) + + +def find_maven(project): + """Prefer the shipped wrapper -- that is what a real user runs.""" + wrapper = project / ("mvnw.cmd" if WINDOWS else "mvnw") + if wrapper.exists(): + return str(wrapper) + found = shutil.which("mvn") + if not found: + raise CanaryFailure("neither the shipped mvnw nor a system mvn is available") + return found + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--base", default="https://cloud.codenameone.com") + parser.add_argument("--target", default="javascript", + help="build target; never use iphone/macos -- those cost 8 credits") + parser.add_argument("--report", help="write a JSON result here") + parser.add_argument("--skip-build", action="store_true", + help="artefact checks only; do not submit a cloud build") + args = parser.parse_args() + global REPORT_PATH + REPORT_PATH = args.report or os.environ.get("CANARY_REPORT") + + email = os.environ.get("CN1_CANARY_EMAIL", "").strip() + password = os.environ.get("CN1_CANARY_PASSWORD", "").strip() + token = os.environ.get("CN1_CANARY_TOKEN", "").strip() + if not email or not password: + raise CanaryFailure( + "CN1_CANARY_EMAIL and CN1_CANARY_PASSWORD are not set; the canary cannot sign in." + ) + if args.target.startswith("iphone") or args.target in ("macos",): + raise CanaryFailure( + f"refusing target '{args.target}': Apple targets cost 8 credits per build " + "and would exhaust the canary account's monthly allowance." + ) + + base = args.base.rstrip("/") + started = time.time() + opener, _ = build_opener() + + log(f"signing in to {base} as {email}") + login(opener, base, email, password) + + with tempfile.TemporaryDirectory(prefix="cn1-canary-") as tmp: + root = Path(tmp) + archive = download_starter(opener, base, args.target, root / "starter.zip") + project, modes = unpack(archive, root / AWKWARD) + log(f"unpacked to {project}") + + check_launcher_bits(project, modes) + version = check_repositories(project) + + if args.skip_build: + log("--skip-build set; stopping after artefact checks") + result = {"ok": True, "stage": "artefact", "cn1Version": version} + else: + if not token: + raise CanaryFailure( + "CN1_CANARY_TOKEN is not set; cannot authenticate the build client headlessly." + ) + mvn = find_maven(project) + seed_token(project, mvn, email, token, version) + # Snapshot first: a free account keeps only its most recent build, + # so "is there a new id" is the only safe way to spot this run's. + known = {b.get("id") for b in list_builds(opener, base)} + code, output = launch(project, args.target) + build = await_cloud_build(opener, base, known, args.target, code, output) + result = { + "ok": True, + "stage": "build", + "cn1Version": version, + "target": args.target, + "buildId": build.get("id"), + "seconds": round(time.time() - started), + } + + if REPORT_PATH: + Path(REPORT_PATH).write_text(json.dumps(result, indent=2)) + log(f"PASS in {round(time.time() - started)}s") + + +if __name__ == "__main__": + try: + main() + except CanaryFailure as failure: + message = str(failure) + print(f"[canary] FAIL: {message}", file=sys.stderr, flush=True) + report = REPORT_PATH or os.environ.get("CANARY_REPORT") + if report: + Path(report).write_text(json.dumps({"ok": False, "error": message}, indent=2)) + sys.exit(1) diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py new file mode 100644 index 00000000000..5342d377869 --- /dev/null +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -0,0 +1,161 @@ +#!/usr/bin/env python3 +"""Self-test for the starter canary's assertions. + +The canary itself talks to the live service, so it cannot run on a PR. These +tests run anywhere and prove each assertion still fires for the breakage it was +written for -- otherwise the canary could quietly degrade into a green check +that verifies nothing, which is the failure mode it exists to end. +""" +import contextlib +import io +import sys +import tempfile +import unittest +import zipfile +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import starter_canary as canary + +GOOD_POM = """ + 7.0.269 + + cn1https://repo.codenameone.com/maven2 + + + cn1phttps://repo.codenameone.com/maven2 + + +""" + +LAUNCHERS = ("build.sh", "run.sh", "mvnw") + + +@contextlib.contextmanager +def fake_builds(rows): + """Stand in for the console build list so the assertions can be tested offline.""" + original = canary.list_builds + canary.list_builds = lambda opener, base: list(rows) + try: + yield + finally: + canary.list_builds = original + + +def make_zip(pom=GOOD_POM, executable=True, root="my-first-app"): + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as zf: + info = zipfile.ZipInfo(f"{root}/pom.xml") + info.external_attr = 0o644 << 16 + zf.writestr(info, pom) + for name in LAUNCHERS: + info = zipfile.ZipInfo(f"{root}/{name}") + info.external_attr = (0o755 if executable else 0o644) << 16 + zf.writestr(info, "#!/bin/sh\necho hi\n") + buffer.seek(0) + return buffer + + +class StarterAssertions(unittest.TestCase): + def unpack(self, buffer): + directory = Path(tempfile.mkdtemp()) + archive = directory / "starter.zip" + archive.write_bytes(buffer.read()) + return canary.unpack(archive, directory / canary.AWKWARD) + + def test_healthy_starter_passes(self): + project, modes = self.unpack(make_zip()) + canary.check_launcher_bits(project, modes) + self.assertEqual(canary.check_repositories(project), "7.0.269") + + def test_non_executable_launchers_fail(self): + """Non-executable launchers: permission denied on the first documented command.""" + project, modes = self.unpack(make_zip(executable=False)) + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_launcher_bits(project, modes) + self.assertIn("non-executable", str(caught.exception)) + self.assertIn("build.sh", str(caught.exception)) + + def test_missing_repositories_fail(self): + """No dependency repository: a pinned release resolves nothing.""" + pom = GOOD_POM.replace("", "").replace("", "") + project, _ = self.unpack(make_zip(pom=pom)) + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_repositories(project) + self.assertIn("repositories", str(caught.exception)) + + def test_missing_plugin_repositories_fail(self): + pom = GOOD_POM.replace("", "").replace("", "") + project, _ = self.unpack(make_zip(pom=pom)) + with self.assertRaises(canary.CanaryFailure): + canary.check_repositories(project) + + def test_repositories_pointing_elsewhere_fail(self): + pom = GOOD_POM.replace("https://repo.codenameone.com/maven2", "https://repo.maven.apache.org/maven2") + project, _ = self.unpack(make_zip(pom=pom)) + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_repositories(project) + self.assertIn("repo.codenameone.com", str(caught.exception)) + + def test_local_jar_without_submission_fails(self): + """A launcher that produces a local jar and says nothing is not a cloud build.""" + with fake_builds([]): + with self.assertRaises(canary.CanaryFailure) as caught: + canary.await_cloud_build( + None, "https://x", set(), "win32", 0, + "BUILD SUCCESS\nBuilding jar: target/app.jar\n", + timeout=0, interval=0) + self.assertIn("never created a build on the server", str(caught.exception)) + + def test_failed_cloud_build_fails(self): + with fake_builds([{"id": "b2", "status": "failed", "submittedAt": 2, + "message": "compilation error"}]): + with self.assertRaises(canary.CanaryFailure) as caught: + canary.await_cloud_build(None, "https://x", {"b1"}, "javascript", 0, "", + timeout=1, interval=0) + self.assertIn("finished as 'failed'", str(caught.exception)) + self.assertIn("compilation error", str(caught.exception)) + + def test_successful_cloud_build_passes(self): + with fake_builds([{"id": "b1", "status": "success", "submittedAt": 1}, + {"id": "b2", "status": "success", "submittedAt": 2}]): + build = canary.await_cloud_build(None, "https://x", {"b1"}, "javascript", 0, "", + timeout=5, interval=0) + self.assertEqual(build["id"], "b2") + + def test_build_stuck_in_queue_fails(self): + with fake_builds([{"id": "b9", "status": "queued", "submittedAt": 9}]): + with self.assertRaises(canary.CanaryFailure) as caught: + canary.await_cloud_build(None, "https://x", set(), "javascript", 0, "", + timeout=0.2, interval=0) + self.assertIn("still 'queued'", str(caught.exception)) + + def test_pre_existing_build_is_not_mistaken_for_ours(self): + """A free account keeps only its latest build; ids, not counts, decide.""" + with fake_builds([{"id": "old", "status": "success", "submittedAt": 1}]): + with self.assertRaises(canary.CanaryFailure) as caught: + canary.await_cloud_build(None, "https://x", {"old"}, "javascript", 0, "", + timeout=0, interval=0) + self.assertIn("never created a build on the server", str(caught.exception)) + + def test_corrupt_archive_is_reported_clearly(self): + """An error page served as starter.zip must not surface as a stack trace.""" + directory = Path(tempfile.mkdtemp()) + archive = directory / "starter.zip" + archive.write_bytes(b"502 Bad Gateway") + with self.assertRaises(canary.CanaryFailure) as caught: + canary.unpack(archive, directory / "out") + self.assertIn("could not be unpacked", str(caught.exception)) + + def test_multiple_roots_rejected(self): + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as zf: + zf.writestr("a/pom.xml", GOOD_POM) + zf.writestr("b/pom.xml", GOOD_POM) + buffer.seek(0) + with self.assertRaises(canary.CanaryFailure): + self.unpack(buffer) + + +if __name__ == "__main__": + unittest.main(verbosity=2) From db90f052d02041554feda436423e59dc80ffbdf1 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:22:44 +0300 Subject: [PATCH 02/10] Read the launcher we were served, and keep the two runners off each other Four corrections from review. The matrix legs were racing. The workflow-level concurrency group serialises separate runs, not the legs inside one run, and both legs share a single build account that allows one build in flight. Beyond a rejected submission, each leg identifies its own build by diffing the account's build list, so one could adopt the other's build and hide a launcher failure specific to its platform. They now run one at a time. On windows-latest the default shell is PowerShell, where a trailing backslash is not a line continuation, so python was handed a stray argument and the following two lines ran as separate commands -- the Windows leg would have failed before it ever touched the downloaded launcher. That step is explicitly bash now. Target names turn out not to be portable between launchers: the project archetype maps `javascript` to a local build and keeps a separate `javascript_cloud`, while the starter the console serves maps `javascript` straight to the cloud target. Rather than hard-code either convention, the canary now reads the launcher it was actually handed and fails immediately if the chosen target would build locally. Without that it would have spent the whole build poll waiting for a submission that was never coming and then blamed the starter. The check is exercised against both conventions in the tests. The Apple guard was matching server-side target strings, not the names the launchers accept, so a dispatch with `ios` sailed past it. It is an allowlist of cheap non-Apple targets now, so `ios`, `ios_release`, `ios_source`, `xcode`, `mac_native` and `mac_catalyst` are all refused, and one added later cannot slip through by not being on a blocklist. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/starter-canary.yml | 12 +++++ scripts/ci/starter-canary/README.md | 12 ++++- scripts/ci/starter-canary/starter_canary.py | 49 +++++++++++++++++-- .../ci/starter-canary/test_starter_canary.py | 35 +++++++++++++ 4 files changed, 104 insertions(+), 4 deletions(-) diff --git a/.github/workflows/starter-canary.yml b/.github/workflows/starter-canary.yml index 70de9f61426..3b29ae08022 100644 --- a/.github/workflows/starter-canary.yml +++ b/.github/workflows/starter-canary.yml @@ -62,6 +62,13 @@ jobs: # One runner must not mask the other: past breakages have been # unix-only (the execute bit) and Windows-only (the batch launcher). fail-fast: false + # The workflow-level concurrency group serialises separate RUNS, not the + # legs inside one run. These two share a single build account, which + # allows one build in flight, and each identifies its own build by + # diffing the account's build list -- so running them together risks a + # rejected submission and, worse, one leg adopting the other's build and + # masking a platform-specific launcher failure. + max-parallel: 1 matrix: os: [ubuntu-latest, windows-latest] runs-on: ${{ matrix.os }} @@ -90,6 +97,11 @@ jobs: CN1_CANARY_PASSWORD: ${{ secrets.CN1_CANARY_PASSWORD }} CN1_CANARY_TOKEN: ${{ secrets.CN1_CANARY_TOKEN }} CANARY_REPORT: ${{ runner.temp }}/canary.json + # Explicitly bash: on windows-latest the default shell is PowerShell, + # where a trailing backslash does not continue a line, so python would + # get a stray argument and the --target/--report lines would be run as + # separate commands. + shell: bash run: | python3 scripts/ci/starter-canary/starter_canary.py \ --target "${{ github.event.inputs.target || 'javascript' }}" \ diff --git a/scripts/ci/starter-canary/README.md b/scripts/ci/starter-canary/README.md index ae099e56770..baf44845b9d 100644 --- a/scripts/ci/starter-canary/README.md +++ b/scripts/ci/starter-canary/README.md @@ -46,7 +46,17 @@ python3 scripts/ci/starter-canary/starter_canary.py --skip-build python3 scripts/ci/starter-canary/starter_canary.py --target javascript ``` -Apple targets cost several times a normal build and the script refuses them. +The canary only accepts cheap, non-Apple targets (`javascript`, +`windows_device`, `windows_desktop`, `linux_device`, `android`, +`android_source`) — an allowlist rather than a blocklist, so a new Apple target +cannot slip through by being added later. + +Target names are also not portable between launchers: the project archetype maps +`javascript` to a **local** build and keeps a separate `javascript_cloud`, while +the starter served by the console maps `javascript` straight to the cloud +target. The canary reads the launcher it was actually handed and fails +immediately if the chosen target would build locally, rather than waiting out +the full build poll for a submission that was never going to happen. ## The assertions diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py index 4709eed8426..b70daf0ff6d 100755 --- a/scripts/ci/starter-canary/starter_canary.py +++ b/scripts/ci/starter-canary/starter_canary.py @@ -53,6 +53,14 @@ # a build failure; keep reproducing that shape here. AWKWARD = "Project O'Brien with spaces" +# An allowlist, not a blocklist. The launchers expose ios, ios_release, +# ios_source, xcode, mac_native and mac_catalyst, all of which need a Mac host +# and cost several times a normal build -- and a blocklist would have to be +# updated every time another one is added. These are the cheap, non-Apple +# targets a canary has any reason to ask for. +CHEAP_TARGETS = ("javascript", "windows_device", "windows_desktop", + "linux_device", "android", "android_source") + class CanaryFailure(RuntimeError): """A user-visible breakage. The message becomes the GitHub issue body.""" @@ -341,6 +349,39 @@ def await_cloud_build(opener, base, known_ids, target, launcher_code, output, ) +def check_target_is_cloud(project, target): + """Confirm the served launcher maps this target to a cloud build. + + Target names are not portable between launchers: the project archetype maps + `javascript` to `local-javascript` and keeps a separate `javascript_cloud`, + while the starter served by the console maps `javascript` straight to the + cloud target. Reading the launcher we were actually handed is the only way + to be sure -- and without this the canary would spend the full build poll + waiting for a build that was never going to be submitted, then report the + starter as broken when the real fault is the target name. + """ + launcher = project / ("build.bat" if WINDOWS else "build.sh") + if not launcher.exists(): + raise CanaryFailure(f"the served starter has no {launcher.name}") + text = launcher.read_text(encoding="utf-8", errors="replace") + marker = f":{target}" if WINDOWS else f"function {target}" + if marker not in text: + offered = re.findall(r"^:([a-z_0-9]+)" if WINDOWS else r"^function ([a-z_0-9]+)", + text, re.MULTILINE) + raise CanaryFailure( + f"the served {launcher.name} has no '{target}' target. It offers: " + f"{', '.join(sorted(set(offered))) or 'nothing recognisable'}." + ) + body = text.split(marker, 1)[1][:400] + if "local-" in body: + raise CanaryFailure( + f"'{target}' maps to a LOCAL build in the served {launcher.name} " + f"(buildTarget contains 'local-'), so it would never submit anything. " + "Point the canary at the launcher's cloud target instead." + ) + log(f"'{target}' is a cloud target in the served {launcher.name}") + + def find_maven(project): """Prefer the shipped wrapper -- that is what a real user runs.""" wrapper = project / ("mvnw.cmd" if WINDOWS else "mvnw") @@ -371,10 +412,11 @@ def main(): raise CanaryFailure( "CN1_CANARY_EMAIL and CN1_CANARY_PASSWORD are not set; the canary cannot sign in." ) - if args.target.startswith("iphone") or args.target in ("macos",): + if args.target not in CHEAP_TARGETS: raise CanaryFailure( - f"refusing target '{args.target}': Apple targets cost 8 credits per build " - "and would exhaust the canary account's monthly allowance." + f"refusing target '{args.target}': the canary only submits cheap, non-Apple " + f"builds ({', '.join(CHEAP_TARGETS)}). Apple targets need a Mac host and cost " + "several times a normal build, which a nightly run would not survive." ) base = args.base.rstrip("/") @@ -392,6 +434,7 @@ def main(): check_launcher_bits(project, modes) version = check_repositories(project) + check_target_is_cloud(project, args.target) if args.skip_build: log("--skip-build set; stopping after artefact checks") diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py index 5342d377869..fdec9fe6671 100644 --- a/scripts/ci/starter-canary/test_starter_canary.py +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -157,5 +157,40 @@ def test_multiple_roots_rejected(self): self.unpack(buffer) +class TargetGuards(unittest.TestCase): + """Target names are not portable between launchers, so read the served one.""" + + CLOUD = 'function javascript {\n "$MVNW" "package" "-Dcodename1.buildTarget=javascript"\n}\n' + LOCAL = 'function javascript {\n "$MVNW" "package" "-Dcodename1.buildTarget=local-javascript"\n}\n' + + def launcher(self, text): + directory = Path(tempfile.mkdtemp()) + (directory / ("build.bat" if canary.WINDOWS else "build.sh")).write_text(text) + return directory + + def test_cloud_target_accepted(self): + canary.check_target_is_cloud(self.launcher(self.CLOUD), "javascript") + + def test_local_target_rejected_before_the_long_poll(self): + """The archetype maps `javascript` to local-javascript; catch it up front.""" + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_target_is_cloud(self.launcher(self.LOCAL), "javascript") + self.assertIn("LOCAL build", str(caught.exception)) + + def test_unknown_target_lists_what_is_offered(self): + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_target_is_cloud(self.launcher(self.CLOUD), "javascript_cloud") + self.assertIn("javascript", str(caught.exception)) + + def test_apple_launcher_targets_are_not_in_the_allowlist(self): + for target in ("ios", "ios_release", "ios_source", "xcode", + "mac_native", "mac_catalyst"): + self.assertNotIn(target, canary.CHEAP_TARGETS, target) + + def test_allowlist_holds_only_cheap_targets(self): + for target in canary.CHEAP_TARGETS: + self.assertFalse(target.startswith(("ios", "mac", "xcode")), target) + + if __name__ == "__main__": unittest.main(verbosity=2) From 510d6bb5ab05e1d5465ea954f840dfb5add09bf0 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:31:13 +0300 Subject: [PATCH 03/10] Call python by the name Windows has, and stop trusting two more assumptions Three more from review. The Windows leg would have died on its first line. Running under Git Bash there is no python3 shim, while setup-python puts `python` on PATH everywhere, so the step exited command-not-found and every Windows run would have reported the starter broken without once touching build.bat. android_source was in the allowlist, and it does not submit anything: its buildTarget is android-source, which generates an Android Studio project on the user's machine. The guard only looked for a `local-` prefix, so it let that through and the canary would have waited out the full build poll before raising a false outage. It now reads the resolved buildTarget and rejects the whole class -- `local-*` and `*-source` alike -- and android_source is off the allowlist beside ios_source and xcode. Authentication was pinned with the wrong property. The starter declares its Maven plugin through cn1.plugin.version, separately from cn1.version; they are equal today but they are separate knobs, and seeding the token with the framework version could resolve a plugin coordinate that was never published. The plugin property is read on its own now and falls back only if it is absent. Both launcher conventions stay covered, and the guards are checked against the real launchers: the served starter accepts javascript and linux_device and refuses android_source, while the archetype refuses javascript and accepts javascript_cloud. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/starter-canary.yml | 7 ++- scripts/ci/starter-canary/README.md | 7 +-- scripts/ci/starter-canary/starter_canary.py | 47 ++++++++++++------- .../ci/starter-canary/test_starter_canary.py | 38 +++++++++++++-- 4 files changed, 73 insertions(+), 26 deletions(-) diff --git a/.github/workflows/starter-canary.yml b/.github/workflows/starter-canary.yml index 3b29ae08022..7a73eb0e729 100644 --- a/.github/workflows/starter-canary.yml +++ b/.github/workflows/starter-canary.yml @@ -52,7 +52,7 @@ jobs: - uses: actions/setup-python@v6 with: python-version: '3.12' - - run: python3 scripts/ci/starter-canary/test_starter_canary.py + - run: python scripts/ci/starter-canary/test_starter_canary.py canary: # Never talk to production from a pull request: forks get no secrets, and a @@ -101,9 +101,12 @@ jobs: # where a trailing backslash does not continue a line, so python would # get a stray argument and the --target/--report lines would be run as # separate commands. + # `python`, not `python3`: this runs under Git Bash on the Windows + # runner, which provides no python3 shim, while setup-python puts + # `python` on PATH for every platform. shell: bash run: | - python3 scripts/ci/starter-canary/starter_canary.py \ + python scripts/ci/starter-canary/starter_canary.py \ --target "${{ github.event.inputs.target || 'javascript' }}" \ ${{ github.event.inputs.skip_build == 'true' && '--skip-build' || '' }} \ --report "${{ runner.temp }}/canary.json" diff --git a/scripts/ci/starter-canary/README.md b/scripts/ci/starter-canary/README.md index baf44845b9d..4b66418b8b2 100644 --- a/scripts/ci/starter-canary/README.md +++ b/scripts/ci/starter-canary/README.md @@ -47,9 +47,10 @@ python3 scripts/ci/starter-canary/starter_canary.py --target javascript ``` The canary only accepts cheap, non-Apple targets (`javascript`, -`windows_device`, `windows_desktop`, `linux_device`, `android`, -`android_source`) — an allowlist rather than a blocklist, so a new Apple target -cannot slip through by being added later. +`windows_device`, `windows_desktop`, `linux_device`, `android`) — an allowlist +rather than a blocklist, so a new Apple target cannot slip through by being +added later. `*_source` and `xcode` are excluded too: they generate an IDE +project locally and submit nothing. Target names are also not portable between launchers: the project archetype maps `javascript` to a **local** build and keeps a separate `javascript_cloud`, while diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py index b70daf0ff6d..3eb7452c920 100755 --- a/scripts/ci/starter-canary/starter_canary.py +++ b/scripts/ci/starter-canary/starter_canary.py @@ -59,7 +59,7 @@ # updated every time another one is added. These are the cheap, non-Apple # targets a canary has any reason to ask for. CHEAP_TARGETS = ("javascript", "windows_device", "windows_desktop", - "linux_device", "android", "android_source") + "linux_device", "android") class CanaryFailure(RuntimeError): @@ -204,11 +204,15 @@ def check_launcher_bits(project, modes): def check_repositories(project): """A pinned release with no repository declared resolves nothing.""" pom = (project / "pom.xml").read_text(encoding="utf-8", errors="replace") - version = None - match = re.search(r"\s*([^<\s]+)\s*", pom) \ - or re.search(r"\s*([^<\s]+)\s*", pom) - if match: - version = match.group(1) + def prop(name): + found = re.search(r"<%s>\s*([^<\s]+)\s*" % (re.escape(name), re.escape(name)), pom) + return found.group(1) if found else None + + version = prop("cn1.version") or prop("codenameone.version") + # The starter declares the Maven plugin through its own property. They are + # equal today, but they are separate knobs, and authenticating with the + # framework version would resolve a plugin coordinate that may not exist. + plugin_version = prop("cn1.plugin.version") or version missing = [ block for block in ("repositories", "pluginRepositories") if f"<{block}>" not in pom @@ -226,11 +230,12 @@ def check_repositories(project): ) if version and version <= "7.0.267": log(f"WARNING: starter pins cn1 {version}, at or below the Maven Central freeze point") - log(f"repository declarations OK (pinned version: {version or 'unknown'})") - return version + log(f"repository declarations OK (cn1 {version or 'unknown'}, " + f"plugin {plugin_version or 'unknown'})") + return version, plugin_version -def seed_token(project, mvn, email, token, version): +def seed_token(project, mvn, email, token, plugin_version): """Headless build-client auth, so no browser OAuth is needed in CI. The goal writes into the java Preferences node the build client reads, so it @@ -238,14 +243,14 @@ def seed_token(project, mvn, email, token, version): An unversioned groupId:artifactId:goal makes Maven resolve LATEST, which is precisely the sort of implicit resolution this canary exists to catch. """ - if not version: + if not plugin_version: raise CanaryFailure( - "could not read the Codename One version out of the served starter pom, " + "could not read the Maven plugin version out of the served starter pom, " "so the build client cannot be authenticated with a pinned plugin." ) run( [mvn, "-B", "-q", - f"com.codenameone:codenameone-maven-plugin:{version}:set-user-token", + f"com.codenameone:codenameone-maven-plugin:{plugin_version}:set-user-token", f"-Dtoken={token}", f"-Duser={email}"], cwd=project, what="cn1:set-user-token", @@ -373,11 +378,17 @@ def check_target_is_cloud(project, target): f"{', '.join(sorted(set(offered))) or 'nothing recognisable'}." ) body = text.split(marker, 1)[1][:400] - if "local-" in body: + built = re.search(r"codename1\.buildTarget=([A-Za-z0-9._-]+)", body) + resolved = built.group(1) if built else "" + # Two shapes never reach the server: an explicitly local target, and a + # *-source target, which generates an Android Studio or Xcode project on + # the user's machine. Both would leave the canary polling for a build that + # was never submitted and then blaming the starter. + if resolved.startswith("local-") or resolved.endswith("-source"): raise CanaryFailure( - f"'{target}' maps to a LOCAL build in the served {launcher.name} " - f"(buildTarget contains 'local-'), so it would never submit anything. " - "Point the canary at the launcher's cloud target instead." + f"'{target}' maps to '{resolved}' in the served {launcher.name}, which " + "builds or generates locally and never submits to the server. Point the " + "canary at one of the launcher's cloud targets instead." ) log(f"'{target}' is a cloud target in the served {launcher.name}") @@ -433,7 +444,7 @@ def main(): log(f"unpacked to {project}") check_launcher_bits(project, modes) - version = check_repositories(project) + version, plugin_version = check_repositories(project) check_target_is_cloud(project, args.target) if args.skip_build: @@ -445,7 +456,7 @@ def main(): "CN1_CANARY_TOKEN is not set; cannot authenticate the build client headlessly." ) mvn = find_maven(project) - seed_token(project, mvn, email, token, version) + seed_token(project, mvn, email, token, plugin_version) # Snapshot first: a free account keeps only its most recent build, # so "is there a new id" is the only safe way to spot this run's. known = {b.get("id") for b in list_builds(opener, base)} diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py index fdec9fe6671..bf0b457b704 100644 --- a/scripts/ci/starter-canary/test_starter_canary.py +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -18,7 +18,10 @@ import starter_canary as canary GOOD_POM = """ - 7.0.269 + + 7.0.269 + 7.0.269 + cn1https://repo.codenameone.com/maven2 @@ -66,7 +69,7 @@ def unpack(self, buffer): def test_healthy_starter_passes(self): project, modes = self.unpack(make_zip()) canary.check_launcher_bits(project, modes) - self.assertEqual(canary.check_repositories(project), "7.0.269") + self.assertEqual(canary.check_repositories(project), ("7.0.269", "7.0.269")) def test_non_executable_launchers_fail(self): """Non-executable launchers: permission denied on the first documented command.""" @@ -157,6 +160,23 @@ def test_multiple_roots_rejected(self): self.unpack(buffer) +class PomProperties(unittest.TestCase): + """The plugin version is its own property and must not be assumed equal.""" + + def test_plugin_version_read_independently(self): + pom = GOOD_POM.replace("7.0.269", + "7.0.271") + directory = Path(tempfile.mkdtemp()) + (directory / "pom.xml").write_text(pom) + self.assertEqual(canary.check_repositories(directory), ("7.0.269", "7.0.271")) + + def test_plugin_version_falls_back_to_framework_version(self): + pom = GOOD_POM.replace("7.0.269", "") + directory = Path(tempfile.mkdtemp()) + (directory / "pom.xml").write_text(pom) + self.assertEqual(canary.check_repositories(directory), ("7.0.269", "7.0.269")) + + class TargetGuards(unittest.TestCase): """Target names are not portable between launchers, so read the served one.""" @@ -175,13 +195,25 @@ def test_local_target_rejected_before_the_long_poll(self): """The archetype maps `javascript` to local-javascript; catch it up front.""" with self.assertRaises(canary.CanaryFailure) as caught: canary.check_target_is_cloud(self.launcher(self.LOCAL), "javascript") - self.assertIn("LOCAL build", str(caught.exception)) + self.assertIn("local-javascript", str(caught.exception)) def test_unknown_target_lists_what_is_offered(self): with self.assertRaises(canary.CanaryFailure) as caught: canary.check_target_is_cloud(self.launcher(self.CLOUD), "javascript_cloud") self.assertIn("javascript", str(caught.exception)) + def test_source_target_rejected_before_the_long_poll(self): + """*-source generates an IDE project locally and submits nothing.""" + text = ('function android_source {\n' + ' "$MVNW" "package" "-Dcodename1.buildTarget=android-source"\n}\n') + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_target_is_cloud(self.launcher(text), "android_source") + self.assertIn("android-source", str(caught.exception)) + + def test_source_targets_are_not_in_the_allowlist(self): + for target in ("android_source", "ios_source", "xcode"): + self.assertNotIn(target, canary.CHEAP_TARGETS, target) + def test_apple_launcher_targets_are_not_in_the_allowlist(self): for target in ("ios", "ios_release", "ios_source", "xcode", "mac_native", "mac_catalyst"): From 29c19ae0f55a690f9a33b5a41fc833b1f5292072 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:39:28 +0300 Subject: [PATCH 04/10] Read the Windows launcher correctly, and fail closed when it cannot be read Review caught that build.bat escapes the separator for cmd, spelling it buildTarget^=, while the regex accepted only a bare `=`. So on Windows nothing ever matched, and because an unreadable target was treated as acceptable the check quietly approved everything it was handed -- including a local target, which would then have cost the full build poll and produced a false outage. The caret is optional now, and an unreadable target is refused rather than assumed innocent: not being able to tell is not evidence that it submits. Verifying that against the real launchers turned up two more faults of the same kind, both mine. The label was matched as a plain substring, so ":ios" matched inside ":ios_source" and "function ios" inside "function ios_source". The check read a neighbouring target's buildTarget and ruled on the wrong one entirely -- `ios` came back as `android-source`. Labels are anchored to a whole line now. And the body was a fixed 400-character window, which runs off the end of a short function into the next one. ios_source only delegates to xcode, so it was being judged on android_source's buildTarget further down the file. The body now stops at the closing brace, or at the next label on Windows, so a delegating target reports that its own target cannot be read instead of borrowing a verdict. Both launchers are now exercised directly in the tests rather than fixtures alone: every allowlisted target resolves as cloud on build.sh and build.bat, android_source is refused as local, and ios_source is refused as unreadable. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/ci/starter-canary/starter_canary.py | 37 ++++++++++++--- .../ci/starter-canary/test_starter_canary.py | 47 +++++++++++++++++++ 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py index 3eb7452c920..5bc146fe2d5 100755 --- a/scripts/ci/starter-canary/starter_canary.py +++ b/scripts/ci/starter-canary/starter_canary.py @@ -369,17 +369,42 @@ def check_target_is_cloud(project, target): if not launcher.exists(): raise CanaryFailure(f"the served starter has no {launcher.name}") text = launcher.read_text(encoding="utf-8", errors="replace") - marker = f":{target}" if WINDOWS else f"function {target}" - if marker not in text: - offered = re.findall(r"^:([a-z_0-9]+)" if WINDOWS else r"^function ([a-z_0-9]+)", + # Anchor the label to a whole line. A plain substring search for ":ios" + # matches inside ":ios_source", and "function ios" inside + # "function ios_source", so the check would read a neighbouring target's + # buildTarget and rule on the wrong one entirely. + marker = (r"^:%s\s*$" % re.escape(target)) if WINDOWS \ + else (r"^function\s+%s\s*\{" % re.escape(target)) + found = re.search(marker, text, re.MULTILINE) + if not found: + offered = re.findall(r"^:([a-z_0-9]+)\s*$" if WINDOWS else r"^function\s+([a-z_0-9]+)\s*\{", text, re.MULTILINE) raise CanaryFailure( f"the served {launcher.name} has no '{target}' target. It offers: " f"{', '.join(sorted(set(offered))) or 'nothing recognisable'}." ) - body = text.split(marker, 1)[1][:400] - built = re.search(r"codename1\.buildTarget=([A-Za-z0-9._-]+)", body) - resolved = built.group(1) if built else "" + # Bound the body to THIS target. A fixed-size window runs past the end of a + # short function into the next one, so a target that merely delegates to + # another (ios_source calls xcode) would be judged on its neighbour's + # buildTarget. Stop at the closing brace, or at the next label on Windows. + rest = text[found.end():] + terminator = re.search(r"^:[a-z_0-9]+\s*$" if WINDOWS else r"^\}\s*$", + rest, re.MULTILINE) + body = rest[:terminator.start()] if terminator else rest + # build.bat escapes the separator for cmd, spelling it `buildTarget^=`, so + # the caret has to be optional -- without it this never matched on Windows + # and the check silently approved every target it was given. + built = re.search(r"codename1\.buildTarget\^?=([A-Za-z0-9._-]+)", body) + if not built: + # Fail closed. Not being able to read the target is not evidence that + # it is a cloud one, and guessing here costs a 30-minute poll and a + # false outage report. + raise CanaryFailure( + f"could not read the buildTarget for '{target}' out of the served " + f"{launcher.name}, so there is no way to tell whether it submits a " + "cloud build. Refusing to run rather than assume it does." + ) + resolved = built.group(1) # Two shapes never reach the server: an explicitly local target, and a # *-source target, which generates an Android Studio or Xcode project on # the user's machine. Both would leave the canary polling for a build that diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py index bf0b457b704..ef1f7373872 100644 --- a/scripts/ci/starter-canary/test_starter_canary.py +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -202,6 +202,53 @@ def test_unknown_target_lists_what_is_offered(self): canary.check_target_is_cloud(self.launcher(self.CLOUD), "javascript_cloud") self.assertIn("javascript", str(caught.exception)) + def test_windows_caret_escaped_target_is_parsed(self): + """build.bat spells it buildTarget^=, which the regex must accept.""" + original = canary.WINDOWS + canary.WINDOWS = True + try: + directory = Path(tempfile.mkdtemp()) + (directory / "build.bat").write_text( + ':javascript\ncall "%MVNW%" package -Dcodename1.buildTarget^=local-javascript -U -e\n') + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_target_is_cloud(directory, "javascript") + self.assertIn("local-javascript", str(caught.exception)) + finally: + canary.WINDOWS = original + + def test_windows_cloud_target_accepted(self): + original = canary.WINDOWS + canary.WINDOWS = True + try: + directory = Path(tempfile.mkdtemp()) + (directory / "build.bat").write_text( + ':javascript\ncall "%MVNW%" package -Dcodename1.buildTarget^=javascript -U -e\n') + canary.check_target_is_cloud(directory, "javascript") + finally: + canary.WINDOWS = original + + def test_prefix_named_neighbour_is_not_matched(self): + """`function ios` must not match inside `function ios_source`.""" + text = ('function ios_source {\n "$MVNW" "-Dcodename1.buildTarget=ios-source"\n}\n' + 'function ios {\n "$MVNW" "-Dcodename1.buildTarget=ios-device"\n}\n') + canary.check_target_is_cloud(self.launcher(text), "ios") + + def test_body_does_not_bleed_into_the_next_target(self): + """A delegating target must not be judged on its neighbour's buildTarget.""" + text = ('function ios_source {\n xcode\n}\n' + 'function android_source {\n "$MVNW" "-Dcodename1.buildTarget=android-source"\n}\n') + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_target_is_cloud(self.launcher(text), "ios_source") + self.assertIn("could not read the buildTarget", str(caught.exception)) + self.assertNotIn("android-source", str(caught.exception)) + + def test_unparseable_target_fails_closed(self): + """Not being able to read the target is not evidence that it is cloud.""" + text = 'function javascript {\n "$MVNW" "package" "-DskipTests"\n}\n' + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_target_is_cloud(self.launcher(text), "javascript") + self.assertIn("could not read the buildTarget", str(caught.exception)) + def test_source_target_rejected_before_the_long_poll(self): """*-source generates an IDE project locally and submits nothing.""" text = ('function android_source {\n' From 90331dc2fc818a8ad0345ab49c7220abda1165d1 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:46:10 +0300 Subject: [PATCH 05/10] Keep the dispatch input out of the shell, and the account out of the report Drops the setup README. Operational runbooks -- how the account is provisioned, what has to be configured where -- do not belong in a public repository. The workflow still names its environment variables because a GitHub Actions workflow cannot reference a secret without naming it, and the script has to read them, but the procedure around them is not published here. The dispatch target was interpolated straight into the step body, where the shell evaluates it before python ever starts. A value like $(...) would have run as a command with all three credentials in the environment, and the target allowlist is enforced far too late to matter. Both inputs now arrive through env and are read as quoted shell variables. A rejected sign-in named the account in the failure text. That text is written to canary.json and copied verbatim into a public tracking issue, and Actions log masking reaches neither artifacts nor issue bodies -- so the first time sign-in broke, which is exactly when this runs, it would have published a configured credential. The message now reports the status and path only. The address is also passed to set-user-token as an argument, so captured output is redacted for it as well as the token, in case a tool echoes its arguments back in an error. The job timeout was 45 minutes while the canary allowed 60 for the launcher and another 30 for the poll, so a slow but healthy build would have been killed before it could write a report -- turning a delay into a reported outage. The two budgets are named constants now, they sum to 50, the job allows 70, and a test asserts the relationship so the next person to raise one finds out here. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/starter-canary.yml | 28 +++++-- scripts/ci/starter-canary/README.md | 78 ------------------- scripts/ci/starter-canary/starter_canary.py | 35 ++++++--- .../ci/starter-canary/test_starter_canary.py | 50 ++++++++++++ 4 files changed, 96 insertions(+), 95 deletions(-) delete mode 100644 scripts/ci/starter-canary/README.md diff --git a/.github/workflows/starter-canary.yml b/.github/workflows/starter-canary.yml index 7a73eb0e729..06fab046076 100644 --- a/.github/workflows/starter-canary.yml +++ b/.github/workflows/starter-canary.yml @@ -72,7 +72,11 @@ jobs: matrix: os: [ubuntu-latest, windows-latest] runs-on: ${{ matrix.os }} - timeout-minutes: 45 + # Must exceed the canary's own budgets (LAUNCH_TIMEOUT 30m + POLL_TIMEOUT + # 20m in starter_canary.py) plus checkout/python/JDK setup. Cut it below + # their sum and the runner kills the job before the canary can write its + # report, turning a slow-but-healthy build into a reported outage. + timeout-minutes: 70 steps: - uses: actions/checkout@v6 @@ -97,19 +101,27 @@ jobs: CN1_CANARY_PASSWORD: ${{ secrets.CN1_CANARY_PASSWORD }} CN1_CANARY_TOKEN: ${{ secrets.CN1_CANARY_TOKEN }} CANARY_REPORT: ${{ runner.temp }}/canary.json - # Explicitly bash: on windows-latest the default shell is PowerShell, - # where a trailing backslash does not continue a line, so python would - # get a stray argument and the --target/--report lines would be run as - # separate commands. + # Through `env`, never interpolated into the script body. A dispatch + # input expanded straight into `run:` is evaluated by the shell before + # python ever sees it, so a value like $(...) would execute with all + # three credentials in the environment. The allowlist runs far too + # late to help with that. + CANARY_TARGET: ${{ github.event.inputs.target || 'javascript' }} + CANARY_SKIP_BUILD: ${{ github.event.inputs.skip_build || 'false' }} # `python`, not `python3`: this runs under Git Bash on the Windows # runner, which provides no python3 shim, while setup-python puts # `python` on PATH for every platform. shell: bash run: | + set -u + skip="" + if [ "$CANARY_SKIP_BUILD" = "true" ]; then + skip="--skip-build" + fi python scripts/ci/starter-canary/starter_canary.py \ - --target "${{ github.event.inputs.target || 'javascript' }}" \ - ${{ github.event.inputs.skip_build == 'true' && '--skip-build' || '' }} \ - --report "${{ runner.temp }}/canary.json" + --target "$CANARY_TARGET" \ + $skip \ + --report "$CANARY_REPORT" - name: Ensure a report exists if: always() diff --git a/scripts/ci/starter-canary/README.md b/scripts/ci/starter-canary/README.md deleted file mode 100644 index 4b66418b8b2..00000000000 --- a/scripts/ci/starter-canary/README.md +++ /dev/null @@ -1,78 +0,0 @@ -# Cloud starter canary - -Nightly end-to-end check that a new user can still reach a first cloud build: -sign in, download the personalised starter from the console, run the shipped -launcher with a real Maven, and require the cloud build to finish — on Linux -and Windows. - -## Why it runs against the live service - -The starter is generated and personalised server-side, then resolves its -dependencies over the network on the user's own machine. Neither of those is -visible to a check that reads the template out of a repository or replaces -Maven with a stub — and both have broken in ways that made a first build -impossible while every offline check stayed green: a generated pom with no -dependency repository, launchers shipped without the execute bit, and a batch -launcher that quietly produced a local jar instead of submitting a build. - -## Setup - -1. Create a dedicated build account for the canary. -2. Add three repository secrets: - - | Secret | Value | - |---|---| - | `CN1_CANARY_EMAIL` | the account's email | - | `CN1_CANARY_PASSWORD` | its console password, for the starter download | - | `CN1_CANARY_TOKEN` | its build token, for headless build-client auth | - - Optionally set the `STARTER_CANARY_ASSIGNEE` repository *variable* to change - who gets assigned the alert issue. -3. Run it once by hand — **Actions → Cloud starter canary → Run workflow** — - before trusting the schedule. - -Until the secrets exist the scheduled run fails and opens the tracking issue. -An unconfigured canary is not a passing one. - -## Running it locally - -```bash -export CN1_CANARY_EMAIL=... CN1_CANARY_PASSWORD=... CN1_CANARY_TOKEN=... - -# artefact checks only, no build submitted -python3 scripts/ci/starter-canary/starter_canary.py --skip-build - -# the full journey -python3 scripts/ci/starter-canary/starter_canary.py --target javascript -``` - -The canary only accepts cheap, non-Apple targets (`javascript`, -`windows_device`, `windows_desktop`, `linux_device`, `android`) — an allowlist -rather than a blocklist, so a new Apple target cannot slip through by being -added later. `*_source` and `xcode` are excluded too: they generate an IDE -project locally and submit nothing. - -Target names are also not portable between launchers: the project archetype maps -`javascript` to a **local** build and keeps a separate `javascript_cloud`, while -the starter served by the console maps `javascript` straight to the cloud -target. The canary reads the launcher it was actually handed and fails -immediately if the chosen target would build locally, rather than waiting out -the full build poll for a submission that was never going to happen. - -## The assertions - -`test_starter_canary.py` runs on every PR touching this directory and proves -each assertion still fires for the breakage it was written for. A canary that -decays into a green check verifying nothing is the failure mode it exists to -end. - -```bash -python3 scripts/ci/starter-canary/test_starter_canary.py -``` - -## When it fails - -The alert job opens or updates a single issue, assigns it, and re-comments at -most once a day while it stays broken, closing it automatically on recovery. -The cause is usually server-side: the starter generator, the vendored starter -template, or the console sign-in redirect. diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py index 5bc146fe2d5..d20dabb9a04 100755 --- a/scripts/ci/starter-canary/starter_canary.py +++ b/scripts/ci/starter-canary/starter_canary.py @@ -58,6 +58,13 @@ # and cost several times a normal build -- and a blocklist would have to be # updated every time another one is added. These are the cheap, non-Apple # targets a canary has any reason to ask for. +# The job timeout in starter-canary.yml must exceed LAUNCH_TIMEOUT + POLL_TIMEOUT +# plus checkout/python/JDK setup. If the runner kills the job first, the canary +# never writes its report and the alert job reports an outage that did not +# happen -- so these two numbers and that one are a single decision. +LAUNCH_TIMEOUT = 1800 # 30 min: mvnw downloads Maven and the toolchain +POLL_TIMEOUT = 1200 # 20 min: waiting for the cloud build to finish + CHEAP_TARGETS = ("javascript", "windows_device", "windows_desktop", "linux_device", "android") @@ -107,9 +114,15 @@ def login(opener, base, email, password): headers={"Content-Type": "application/x-www-form-urlencoded"}, ) if status not in (200, 302) or "error" in urllib.parse.urlparse(final).query: + # Never name the account. This message is written to canary.json, and + # the alert job copies it verbatim into a tracking issue -- Actions log + # masking does not reach artifacts or issue bodies, so putting the + # address here would publish a configured credential the first time + # sign-in broke, which is precisely when this runs. raise CanaryFailure( - f"form login as {email} failed (HTTP {status}, landed on {final}). " - "Either the canary credentials are wrong or the sign-in path is broken." + f"the canary account could not sign in (HTTP {status}, landed on " + f"{urllib.parse.urlparse(final).path}). Either the configured " + "credentials are wrong or the sign-in path is broken." ) return final @@ -254,18 +267,22 @@ def seed_token(project, mvn, email, token, plugin_version): f"-Dtoken={token}", f"-Duser={email}"], cwd=project, what="cn1:set-user-token", - secret=token, + secrets=(token, email), ) log("seeded build-client token") -def run(command, cwd, what, timeout=3600, secret=None, check=True): +def run(command, cwd, what, timeout=3600, secrets=(), check=True): result = subprocess.run( command, cwd=str(cwd), capture_output=True, text=True, timeout=timeout ) output = (result.stdout or "") + (result.stderr or "") - if secret: - output = output.replace(secret, "***") + # Redact before this can reach a report, and so an issue body. The account + # address is passed to the goal as an argument, so a tool that echoes its + # arguments back in an error would otherwise carry it straight out. + for value in secrets: + if value: + output = output.replace(value, "***") if check and result.returncode != 0: raise CanaryFailure( f"{what} failed with exit {result.returncode}:\n{tail(output)}" @@ -290,7 +307,7 @@ def launch(project, target): env_note = f"{'build.bat' if WINDOWS else './build.sh'} {target}" log(f"running {env_note} (this downloads Maven and the CN1 toolchain; several minutes)") code, output = run( - command, cwd=project, what=env_note, timeout=3600, check=False + command, cwd=project, what=env_note, timeout=LAUNCH_TIMEOUT, check=False ) return code, output @@ -317,7 +334,7 @@ def list_builds(opener, base): def await_cloud_build(opener, base, known_ids, target, launcher_code, output, - timeout=1800, interval=20): + timeout=POLL_TIMEOUT, interval=20): """Wait for a build this run submitted to reach a terminal state.""" deadline = time.time() + timeout seen = None @@ -459,7 +476,7 @@ def main(): started = time.time() opener, _ = build_opener() - log(f"signing in to {base} as {email}") + log(f"signing in to {base}") # the account is never named in output login(opener, base, email, password) with tempfile.TemporaryDirectory(prefix="cn1-canary-") as tmp: diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py index ef1f7373872..ef81f5c3bc0 100644 --- a/scripts/ci/starter-canary/test_starter_canary.py +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -160,6 +160,56 @@ def test_multiple_roots_rejected(self): self.unpack(buffer) +class Redaction(unittest.TestCase): + """Failure text reaches a public tracking issue, so it must name no account.""" + + def test_login_failure_does_not_name_the_account(self): + class Response: + """Spring bounces a rejected sign-in back to /login?error.""" + + def __init__(self, url): + self.status, self.headers, self.url = 200, {}, url + + def read(self): + return b'' + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + class Opener: + def __init__(self): + self.calls = 0 + + def open(self, request, timeout=0): + self.calls += 1 + return Response("https://x/login" if self.calls == 1 + else "https://x/login?error") + + with self.assertRaises(canary.CanaryFailure) as caught: + canary.login(Opener(), "https://x", "secret-account@example.com", "pw") + message = str(caught.exception) + self.assertNotIn("secret-account@example.com", message) + self.assertIn("could not sign in", message) + + def test_run_redacts_every_supplied_credential(self): + code, output = canary.run( + [sys.executable, "-c", "print('tok-abc123 user@example.com ok')"], + cwd=".", what="probe", secrets=("tok-abc123", "user@example.com")) + self.assertNotIn("tok-abc123", output) + self.assertNotIn("user@example.com", output) + self.assertIn("***", output) + + +class Budgets(unittest.TestCase): + def test_internal_budgets_fit_the_documented_job_timeout(self): + """The workflow allows 70 minutes; the canary must finish inside it.""" + total_minutes = (canary.LAUNCH_TIMEOUT + canary.POLL_TIMEOUT) / 60 + self.assertLess(total_minutes, 70, "job timeout-minutes must exceed this") + + class PomProperties(unittest.TestCase): """The plugin version is its own property and must not be assumed equal.""" From 7c5013a9ab8b8bfb2bb6077a8638fae45bf376eb Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 16 Sep 2026 05:09:28 +0300 Subject: [PATCH 06/10] Count the token-seeding phase in the job budget too Bounding the launcher and the poll left a third blocking phase uncounted: set-user-token went through run()'s 3600-second default, so a Maven that stalled resolving it could spend an hour before the thirty-minute launcher and twenty-minute poll had even started, and the runner would kill the job before python could write the report that says what went wrong. That phase is SEED_TIMEOUT now, ten minutes, and the three budgets sum to sixty against a seventy-minute job. run() no longer has a default timeout at all. An unbounded default is what let a phase escape the arithmetic in the first place, so every call site has to name a number that the budget test can add up, and the test asserts there is no default to fall back to. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/ci/starter-canary/starter_canary.py | 4 +++- .../ci/starter-canary/test_starter_canary.py | 20 ++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py index d20dabb9a04..33ef3f2725f 100755 --- a/scripts/ci/starter-canary/starter_canary.py +++ b/scripts/ci/starter-canary/starter_canary.py @@ -62,6 +62,7 @@ # plus checkout/python/JDK setup. If the runner kills the job first, the canary # never writes its report and the alert job reports an outage that did not # happen -- so these two numbers and that one are a single decision. +SEED_TIMEOUT = 600 # 10 min: resolving and running one small goal LAUNCH_TIMEOUT = 1800 # 30 min: mvnw downloads Maven and the toolchain POLL_TIMEOUT = 1200 # 20 min: waiting for the cloud build to finish @@ -267,12 +268,13 @@ def seed_token(project, mvn, email, token, plugin_version): f"-Dtoken={token}", f"-Duser={email}"], cwd=project, what="cn1:set-user-token", + timeout=SEED_TIMEOUT, secrets=(token, email), ) log("seeded build-client token") -def run(command, cwd, what, timeout=3600, secrets=(), check=True): +def run(command, cwd, what, timeout, secrets=(), check=True): result = subprocess.run( command, cwd=str(cwd), capture_output=True, text=True, timeout=timeout ) diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py index ef81f5c3bc0..98f33ff1fdb 100644 --- a/scripts/ci/starter-canary/test_starter_canary.py +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -197,17 +197,27 @@ def open(self, request, timeout=0): def test_run_redacts_every_supplied_credential(self): code, output = canary.run( [sys.executable, "-c", "print('tok-abc123 user@example.com ok')"], - cwd=".", what="probe", secrets=("tok-abc123", "user@example.com")) + cwd=".", what="probe", timeout=60, + secrets=("tok-abc123", "user@example.com")) self.assertNotIn("tok-abc123", output) self.assertNotIn("user@example.com", output) self.assertIn("***", output) class Budgets(unittest.TestCase): - def test_internal_budgets_fit_the_documented_job_timeout(self): - """The workflow allows 70 minutes; the canary must finish inside it.""" - total_minutes = (canary.LAUNCH_TIMEOUT + canary.POLL_TIMEOUT) / 60 - self.assertLess(total_minutes, 70, "job timeout-minutes must exceed this") + JOB_TIMEOUT_MINUTES = 70 # starter-canary.yml + + def test_every_phase_fits_the_job_timeout(self): + """Each blocking phase must be counted, or the runner kills the job first.""" + total = (canary.SEED_TIMEOUT + canary.LAUNCH_TIMEOUT + canary.POLL_TIMEOUT) / 60 + self.assertLess(total, self.JOB_TIMEOUT_MINUTES, + "raise timeout-minutes in starter-canary.yml to cover this") + + def test_run_requires_an_explicit_timeout(self): + """No unbounded default: an uncounted phase is how the budget drifted.""" + import inspect + parameter = inspect.signature(canary.run).parameters["timeout"] + self.assertIs(parameter.default, inspect.Parameter.empty) class PomProperties(unittest.TestCase): From 3f08ec13c8366f724641e6dc612944a53ddeed78 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 16 Sep 2026 05:47:19 +0300 Subject: [PATCH 07/10] Mint the build token per run instead of storing one The build client does not authenticate with the account's app token. It stores the JWT that /appsec/7.0/set-user mints and /poll-user hands back, so seeding it with the app token left it unauthenticated -- and what the client does then is worth recording: it printed a browser login URL, said it would wait five minutes, did not wait, reported "your build was submitted", and exited BUILD SUCCESSFUL after six seconds. No build existed on the server. Proven against production: the account's build list was empty afterwards. That is the local-jar failure again in a different component, and it is exactly why success is read from the console's build list rather than from launcher output. The canary called it correctly with no change: no new build id, so no build. The token is now minted per run from the session the canary already holds, the way the tooling itself obtains one. There is no long-lived build credential in repository secrets, nothing to rotate when it expires, and one fewer secret to configure -- email and password are enough. An unexpected exception also wrote no report at all, so a crash reached the alert job as "no report" rather than as what broke. Every exception is captured now, with the type and message in the report. Verified end to end against production: sign in, download the personalised starter, mint a token, run the shipped ./build.sh javascript, and wait for the cloud build to finish -- build 02ad7a37 succeeded in 282 seconds, comfortably inside the ten/thirty/twenty minute phase budgets. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/starter-canary.yml | 1 - scripts/ci/starter-canary/starter_canary.py | 47 ++++++++++++++--- .../ci/starter-canary/test_starter_canary.py | 51 +++++++++++++++++++ 3 files changed, 91 insertions(+), 8 deletions(-) diff --git a/.github/workflows/starter-canary.yml b/.github/workflows/starter-canary.yml index 06fab046076..47ead78b10a 100644 --- a/.github/workflows/starter-canary.yml +++ b/.github/workflows/starter-canary.yml @@ -99,7 +99,6 @@ jobs: env: CN1_CANARY_EMAIL: ${{ secrets.CN1_CANARY_EMAIL }} CN1_CANARY_PASSWORD: ${{ secrets.CN1_CANARY_PASSWORD }} - CN1_CANARY_TOKEN: ${{ secrets.CN1_CANARY_TOKEN }} CANARY_REPORT: ${{ runner.temp }}/canary.json # Through `env`, never interpolated into the script body. A dispatch # input expanded straight into `run:` is evaluated by the shell before diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py index 33ef3f2725f..4516fc18f2a 100755 --- a/scripts/ci/starter-canary/starter_canary.py +++ b/scripts/ci/starter-canary/starter_canary.py @@ -43,6 +43,7 @@ import urllib.error import urllib.parse import urllib.request +import uuid import zipfile from http.cookiejar import CookieJar from pathlib import Path @@ -249,6 +250,39 @@ def prop(name): return version, plugin_version +def mint_build_token(opener, base): + """Get a build-client token the way the tooling does, from our own session. + + The build client does not authenticate with the account's app token. It + stores the JWT that `/appsec/7.0/set-user` mints and `/poll-user` hands back, + so that is what has to be seeded -- feeding it the app token instead leaves + it unauthenticated, and the client then prints a browser login URL, does not + wait for it, reports "your build was submitted", and exits 0 without having + submitted anything. + + Minting per run rather than storing one also means there is no long-lived + build credential in repository secrets, and nothing to rotate when it + expires. + """ + key = str(uuid.uuid4()) + redirect = urllib.parse.quote(f"{base}/loggedIn.html", safe="") + status, _, _, _ = fetch( + opener, f"{base}/appsec/7.0/set-user?loginKey={key}&redirect={redirect}&ver=2") + if status != 200: + raise CanaryFailure( + f"set-user returned HTTP {status}; the console session did not carry " + "into the build-client login, so no token could be minted." + ) + status, body, _, _ = fetch(opener, f"{base}/poll-user?ver=2&loginKey={key}") + if status != 200: + raise CanaryFailure(f"poll-user returned HTTP {status}; no build token was issued.") + lines = body.decode("utf-8", "replace").strip().splitlines() + if not lines or not lines[0].strip(): + raise CanaryFailure("poll-user returned no build token.") + log("minted a build-client token from the console session") + return lines[0].strip() + + def seed_token(project, mvn, email, token, plugin_version): """Headless build-client auth, so no browser OAuth is needed in CI. @@ -462,7 +496,6 @@ def main(): email = os.environ.get("CN1_CANARY_EMAIL", "").strip() password = os.environ.get("CN1_CANARY_PASSWORD", "").strip() - token = os.environ.get("CN1_CANARY_TOKEN", "").strip() if not email or not password: raise CanaryFailure( "CN1_CANARY_EMAIL and CN1_CANARY_PASSWORD are not set; the canary cannot sign in." @@ -495,10 +528,7 @@ def main(): log("--skip-build set; stopping after artefact checks") result = {"ok": True, "stage": "artefact", "cn1Version": version} else: - if not token: - raise CanaryFailure( - "CN1_CANARY_TOKEN is not set; cannot authenticate the build client headlessly." - ) + token = mint_build_token(opener, base) mvn = find_maven(project) seed_token(project, mvn, email, token, plugin_version) # Snapshot first: a free account keeps only its most recent build, @@ -523,8 +553,11 @@ def main(): if __name__ == "__main__": try: main() - except CanaryFailure as failure: - message = str(failure) + except Exception as failure: # noqa: BLE001 - a crash must still be reported + message = str(failure) if isinstance(failure, CanaryFailure) else ( + f"the canary crashed before it could finish: " + f"{type(failure).__name__}: {failure}" + ) print(f"[canary] FAIL: {message}", file=sys.stderr, flush=True) report = REPORT_PATH or os.environ.get("CANARY_REPORT") if report: diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py index 98f33ff1fdb..1d1fa4599df 100644 --- a/scripts/ci/starter-canary/test_starter_canary.py +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -204,6 +204,57 @@ def test_run_redacts_every_supplied_credential(self): self.assertIn("***", output) +class TokenMinting(unittest.TestCase): + """The build client wants the minted JWT, not the account's app token.""" + + class Response: + def __init__(self, body=b"", status=200): + self.status, self.headers, self.url = status, {}, "https://x" + self._body = body + + def read(self): + return self._body + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def opener(self, *responses): + outer = self + + class Opener: + def __init__(self): + self.calls, self.urls = 0, [] + + def open(self, request, timeout=0): + self.urls.append(request.full_url) + response = responses[min(self.calls, len(responses) - 1)] + self.calls += 1 + return response + + return Opener() + + def test_token_is_the_first_line_of_poll_user(self): + opener = self.opener(self.Response(b""), + self.Response(b"jwt-value\nuser@example.com\n")) + self.assertEqual(canary.mint_build_token(opener, "https://x"), "jwt-value") + self.assertIn("set-user", opener.urls[0]) + self.assertIn("poll-user", opener.urls[1]) + + def test_empty_poll_response_is_a_failure(self): + opener = self.opener(self.Response(b""), self.Response(b"\n")) + with self.assertRaises(canary.CanaryFailure): + canary.mint_build_token(opener, "https://x") + + def test_set_user_rejection_is_reported(self): + opener = self.opener(self.Response(b"", status=401)) + with self.assertRaises(canary.CanaryFailure) as caught: + canary.mint_build_token(opener, "https://x") + self.assertIn("set-user", str(caught.exception)) + + class Budgets(unittest.TestCase): JOB_TIMEOUT_MINUTES = 70 # starter-canary.yml From d4fee2f0345b63b69cd0c87bec4d2cd7fb2d9831 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 16 Sep 2026 06:00:16 +0300 Subject: [PATCH 08/10] Keep credentials out of a timeout, and take the whole process tree with it A timeout on the token-seeding step did not go through the redaction at all. subprocess raises TimeoutExpired before any output is returned, and that exception stringifies the entire command line -- which carries the minted token and the account address -- straight into the report the alert job copies into a public issue. The exception is caught now and never surfaced; the message says which phase timed out and shows redacted output. Killing a timed-out launcher only killed the launcher. It is a shell that execs mvnw, which execs a JVM, and that JVM can go on to submit a build minutes after its own leg has already failed -- which the next serialised leg would then see as a build id it did not create, recreating exactly the cross-leg confusion max-parallel exists to prevent. Each phase runs in its own process group and a timeout takes the group down, with the drain afterwards bounded so a survivor holding the pipe cannot hang the phase we just gave up on. An artefact-only dispatch could also close an outage. With skip_build the run reports ok, and the alert job read only that flag, so a diagnostic run would have closed an open issue and announced that both platforms completed a cloud build without one having been submitted. Recovery now requires stage "build"; an artefact-only run is inconclusive and leaves the issue exactly as it found it. Verified end to end against production after the rewrite, because this replaced the function every external command goes through: build bf1cfcf1 succeeded in 283 seconds, against 282 before it. The timeout tests are real rather than mocked -- one asserts a credential cannot appear in a timeout message, the other spawns a grandchild and asserts it does not outlive the kill. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/starter-canary.yml | 26 +++++-- scripts/ci/starter-canary/starter_canary.py | 67 +++++++++++++++---- .../ci/starter-canary/test_starter_canary.py | 36 +++++++++- 3 files changed, 111 insertions(+), 18 deletions(-) diff --git a/.github/workflows/starter-canary.yml b/.github/workflows/starter-canary.yml index 47ead78b10a..eda99ea2ed5 100644 --- a/.github/workflows/starter-canary.yml +++ b/.github/workflows/starter-canary.yml @@ -176,21 +176,27 @@ jobs: const read = (dir, label) => { const file = `reports/canary-${dir}/canary.json`; if (!fs.existsSync(file)) { - return `${label}: the canary job produced no report (it did not run, or the runner died).`; + return { problem: `${label}: the canary job produced no report (it did not run, or the runner died).` }; } const raw = fs.readFileSync(file, 'utf8'); try { const data = JSON.parse(raw); - return data.ok ? null : `${label}: ${data.error}`; + if (!data.ok) return { problem: `${label}: ${data.error}` }; + // An artefact-only dispatch proves nothing about cloud builds. + // Treating it as recovery would close an open outage and claim + // both platforms built, without a build having been submitted. + return { inconclusive: data.stage !== 'build' }; } catch (e) { - return `${label}: unreadable canary report -- ${raw.slice(0, 300)}`; + return { problem: `${label}: unreadable canary report -- ${raw.slice(0, 300)}` }; } }; - const problems = [ + const results = [ read('ubuntu-latest', 'Linux'), read('windows-latest', 'Windows'), - ].filter(Boolean); + ]; + const problems = results.map(r => r.problem).filter(Boolean); + const inconclusive = problems.length === 0 && results.some(r => r.inconclusive); const issues = await github.paginate(github.rest.issues.listForRepo, { owner, repo, state: 'open', per_page: 100, @@ -201,6 +207,16 @@ jobs: (candidate.body || '').includes(marker) ); + if (inconclusive) { + // Leave any open issue exactly as it is: not proven broken, and + // certainly not proven fixed. + await core.summary + .addHeading('Cloud starter canary') + .addRaw('Artefact checks passed; no cloud build was submitted, so this run neither confirms nor clears an outage.') + .write(); + return; + } + if (problems.length === 0) { await core.summary.addHeading('Cloud starter canary').addRaw('Healthy').write(); if (issue) { diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py index 4516fc18f2a..1ab01ac381a 100755 --- a/scripts/ci/starter-canary/starter_canary.py +++ b/scripts/ci/starter-canary/starter_canary.py @@ -34,6 +34,7 @@ import json import os import re +import signal import shutil import stat import subprocess @@ -308,22 +309,64 @@ def seed_token(project, mvn, email, token, plugin_version): log("seeded build-client token") -def run(command, cwd, what, timeout, secrets=(), check=True): - result = subprocess.run( - command, cwd=str(cwd), capture_output=True, text=True, timeout=timeout - ) - output = (result.stdout or "") + (result.stderr or "") - # Redact before this can reach a report, and so an issue body. The account - # address is passed to the goal as an argument, so a tool that echoes its - # arguments back in an error would otherwise carry it straight out. +def redact(text, secrets): for value in secrets: if value: - output = output.replace(value, "***") - if check and result.returncode != 0: + text = text.replace(value, "***") + return text + + +def terminate_tree(process): + """Kill the launcher AND everything it spawned. + + subprocess kills only the direct child, but the launcher is a shell that + execs mvnw, which execs a JVM. Left alive, that JVM can submit a build + minutes after this leg has already failed -- and the next serialised leg + would then see a build id it did not create, which is exactly the + cross-leg confusion max-parallel is there to prevent. + """ + try: + if WINDOWS: + subprocess.run(["taskkill", "/F", "/T", "/PID", str(process.pid)], + capture_output=True, timeout=60) + else: + os.killpg(os.getpgid(process.pid), signal.SIGKILL) + except Exception: # noqa: BLE001 - best effort; the report matters more + process.kill() + + +def run(command, cwd, what, timeout, secrets=(), check=True): + # Its own process group, so a timeout can take the whole tree down. + extra = {"creationflags": subprocess.CREATE_NEW_PROCESS_GROUP} if WINDOWS \ + else {"start_new_session": True} + process = subprocess.Popen( + command, cwd=str(cwd), stdout=subprocess.PIPE, stderr=subprocess.STDOUT, + text=True, **extra + ) + try: + output, _ = process.communicate(timeout=timeout) + except subprocess.TimeoutExpired as expired: + terminate_tree(process) + # Bounded: if anything in the tree survived and still holds the pipe, + # draining it must not hang the phase we just gave up on. + try: + partial, _ = process.communicate(timeout=30) + except subprocess.TimeoutExpired: + process.kill() + partial = "" + # Never surface the exception itself. Its str() carries the whole + # command line, which holds the minted token and the account address, + # and this text is written to the report and copied into a public issue. + raise CanaryFailure( + f"{what} did not finish within {timeout}s and was terminated.\n\n" + + tail(redact((expired.output or "") + (partial or ""), secrets)) + ) from None + output = redact(output or "", secrets) + if check and process.returncode != 0: raise CanaryFailure( - f"{what} failed with exit {result.returncode}:\n{tail(output)}" + f"{what} failed with exit {process.returncode}:\n{tail(output)}" ) - return result.returncode, output + return process.returncode, output def tail(text, lines=40): diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py index 1d1fa4599df..a4719b3241a 100644 --- a/scripts/ci/starter-canary/test_starter_canary.py +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -10,6 +10,7 @@ import io import sys import tempfile +import time import unittest import zipfile from pathlib import Path @@ -222,7 +223,6 @@ def __exit__(self, *exc): return False def opener(self, *responses): - outer = self class Opener: def __init__(self): @@ -255,6 +255,40 @@ def test_set_user_rejection_is_reported(self): self.assertIn("set-user", str(caught.exception)) +class TimeoutHandling(unittest.TestCase): + """A timeout must not carry the command line into a public issue.""" + + def test_timeout_message_hides_the_credentials_in_the_command(self): + secret_token = "jwt-should-not-appear" + secret_email = "account-should-not-appear@example.com" + with self.assertRaises(canary.CanaryFailure) as caught: + canary.run( + [sys.executable, "-c", + f"import time; print('{secret_token} {secret_email}'); time.sleep(30)"], + cwd=".", what="probe", timeout=1, + secrets=(secret_token, secret_email)) + message = str(caught.exception) + self.assertNotIn(secret_token, message) + self.assertNotIn(secret_email, message) + self.assertIn("did not finish within", message) + + def test_timeout_kills_the_whole_process_tree(self): + """A surviving grandchild could submit a build after this leg failed.""" + marker = Path(tempfile.mkdtemp()) / "grandchild-survived" + # Parent spawns a child that outlives it and would write the marker. + script = ( + "import subprocess,sys,time;" + f"subprocess.Popen([sys.executable,'-c',\"import time;time.sleep(4);" + f"open(r'{marker}','w').write('x')\"]);" + "time.sleep(30)" + ) + with self.assertRaises(canary.CanaryFailure): + canary.run([sys.executable, "-c", script], cwd=".", what="probe", timeout=1) + time.sleep(6) + self.assertFalse(marker.exists(), + "a spawned grandchild outlived the timeout") + + class Budgets(unittest.TestCase): JOB_TIMEOUT_MINUTES = 70 # starter-canary.yml From 9e32723a04d817d6a9e727753dedfd4ace2bf2a5 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 16 Sep 2026 06:13:43 +0300 Subject: [PATCH 09/10] Give the build no credentials, and police what a target resolves to Every subprocess inherited the whole environment, and one of the things in it was the account password. Maven resolves artifacts over the network and runs plugin code out of them, so a compromised dependency anywhere in the starter's tree could read a long-lived credential that sign-in had already finished with. Children get a sanitised environment now, without the canary credentials or the Actions tokens. The token still reaches set-user-token as an argument, so nothing has to travel in the environment at all. The Apple guard only ever checked the name the user asked for. If the served launcher regressed so javascript pointed at ios-device, the allowlist would wave it through and both legs would submit an Apple build every night at eight credits each -- roughly half the monthly allowance a week. The resolved buildTarget is checked against the same policy now, independently of the name that reached it. That change caught one of the existing tests asserting `ios` passes as a cloud target. It does resolve to one, but the test was written to prove `function ios` does not match inside `function ios_source`, and used Apple names only incidentally. It now makes the same point with android and android_source, so it still tests the anchoring without depending on a target the policy refuses. Loosening the policy to keep the assertion green would have had it backwards. Verified against production again, because this changed the environment every child runs in and no unit test proves Maven still works without those variables: build 119f38ca succeeded in 283 seconds, matching the previous two runs. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/ci/starter-canary/starter_canary.py | 32 ++++++++- .../ci/starter-canary/test_starter_canary.py | 70 +++++++++++++++++-- 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py index 1ab01ac381a..19a945cbd12 100755 --- a/scripts/ci/starter-canary/starter_canary.py +++ b/scripts/ci/starter-canary/starter_canary.py @@ -309,6 +309,28 @@ def seed_token(project, mvn, email, token, plugin_version): log("seeded build-client token") +# Anything a child process has no business seeing. Maven resolves artifacts +# over the network and runs plugin code from them, so every variable exported +# here is readable by code we did not write. +SENSITIVE_ENV = ("CN1_CANARY_PASSWORD", "CN1_CANARY_EMAIL", "CN1_CANARY_TOKEN", + "CANARY_REPORT", "GITHUB_TOKEN", "ACTIONS_RUNTIME_TOKEN", + "ACTIONS_ID_TOKEN_REQUEST_TOKEN", "ACTIONS_ID_TOKEN_REQUEST_URL") + + +def child_environment(): + """The environment a build is allowed to run in. + + The account password is not needed after sign-in, and the token is passed to + the goal as an argument rather than through the environment -- so nothing + here has to carry a credential, and a compromised dependency resolved by the + starter has nothing to exfiltrate. + """ + env = dict(os.environ) + for name in SENSITIVE_ENV: + env.pop(name, None) + return env + + def redact(text, secrets): for value in secrets: if value: @@ -341,7 +363,7 @@ def run(command, cwd, what, timeout, secrets=(), check=True): else {"start_new_session": True} process = subprocess.Popen( command, cwd=str(cwd), stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - text=True, **extra + text=True, env=child_environment(), **extra ) try: output, _ = process.communicate(timeout=timeout) @@ -505,6 +527,14 @@ def check_target_is_cloud(project, target): # *-source target, which generates an Android Studio or Xcode project on # the user's machine. Both would leave the canary polling for a build that # was never submitted and then blaming the starter. + expensive = ("ios", "iphone", "ipad", "mac", "catalyst", "xcode", "watch", "tv") + hit = next((m for m in expensive if m in resolved.lower()), None) + if hit: + raise CanaryFailure( + f"'{target}' resolves to '{resolved}' in the served {launcher.name}, which " + f"looks like an Apple target ('{hit}'). Those need a Mac host and cost several " + "times a normal build; the canary refuses them however they are reached." + ) if resolved.startswith("local-") or resolved.endswith("-source"): raise CanaryFailure( f"'{target}' maps to '{resolved}' in the served {launcher.name}, which " diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py index a4719b3241a..77cca4b3256 100644 --- a/scripts/ci/starter-canary/test_starter_canary.py +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -289,6 +289,64 @@ def test_timeout_kills_the_whole_process_tree(self): "a spawned grandchild outlived the timeout") +class ChildEnvironment(unittest.TestCase): + """Maven runs plugin code resolved over the network; it gets no credentials.""" + + def test_credentials_are_stripped_from_the_child_environment(self): + import os + for name in ("CN1_CANARY_PASSWORD", "CN1_CANARY_EMAIL", "GITHUB_TOKEN"): + os.environ[name] = "must-not-be-inherited" + try: + env = canary.child_environment() + for name in canary.SENSITIVE_ENV: + self.assertNotIn(name, env, name) + self.assertIn("PATH", env, "the child still needs an ordinary environment") + finally: + for name in ("CN1_CANARY_PASSWORD", "CN1_CANARY_EMAIL", "GITHUB_TOKEN"): + os.environ.pop(name, None) + + def test_a_spawned_process_cannot_read_the_password(self): + import os + os.environ["CN1_CANARY_PASSWORD"] = "leaked-password-value" + try: + code, output = canary.run( + [sys.executable, "-c", + "import os;print('SEEN:'+os.environ.get('CN1_CANARY_PASSWORD','absent'))"], + cwd=".", what="probe", timeout=60) + self.assertIn("SEEN:absent", output) + self.assertNotIn("leaked-password-value", output) + finally: + os.environ.pop("CN1_CANARY_PASSWORD", None) + + +class ResolvedTargetPolicy(unittest.TestCase): + """The allowlist guards the name; this guards what the name resolves to.""" + + def launcher(self, text): + directory = Path(tempfile.mkdtemp()) + (directory / ("build.bat" if canary.WINDOWS else "build.sh")).write_text(text) + return directory + + def test_allowed_name_resolving_to_an_apple_target_is_refused(self): + text = ('function javascript {\n' + ' "$MVNW" "package" "-Dcodename1.buildTarget=ios-device"\n}\n') + with self.assertRaises(canary.CanaryFailure) as caught: + canary.check_target_is_cloud(self.launcher(text), "javascript") + self.assertIn("Apple target", str(caught.exception)) + + def test_mac_native_regression_is_refused(self): + text = ('function javascript {\n' + ' "$MVNW" "package" "-Dcodename1.buildTarget=mac-os-x-native"\n}\n') + with self.assertRaises(canary.CanaryFailure): + canary.check_target_is_cloud(self.launcher(text), "javascript") + + def test_ordinary_cheap_targets_still_pass(self): + for resolved in ("javascript", "windows-device", "linux-device", "android-device"): + text = ('function t {\n' + f' "$MVNW" "package" "-Dcodename1.buildTarget={resolved}"\n}}\n') + canary.check_target_is_cloud(self.launcher(text), "t") + + class Budgets(unittest.TestCase): JOB_TIMEOUT_MINUTES = 70 # starter-canary.yml @@ -373,10 +431,14 @@ def test_windows_cloud_target_accepted(self): canary.WINDOWS = original def test_prefix_named_neighbour_is_not_matched(self): - """`function ios` must not match inside `function ios_source`.""" - text = ('function ios_source {\n "$MVNW" "-Dcodename1.buildTarget=ios-source"\n}\n' - 'function ios {\n "$MVNW" "-Dcodename1.buildTarget=ios-device"\n}\n') - canary.check_target_is_cloud(self.launcher(text), "ios") + """`function android` must not match inside `function android_source`. + + Uses a cheap pair on purpose: an Apple pair would now be refused by the + resolved-target policy before the anchoring could be observed. + """ + text = ('function android_source {\n "$MVNW" "-Dcodename1.buildTarget=android-source"\n}\n' + 'function android {\n "$MVNW" "-Dcodename1.buildTarget=android-device"\n}\n') + canary.check_target_is_cloud(self.launcher(text), "android") def test_body_does_not_bleed_into_the_next_target(self): """A delegating target must not be judged on its neighbour's buildTarget.""" From 46bb8f9749ed3d0c5677220b211f040188caaf8c Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 16 Sep 2026 06:26:26 +0300 Subject: [PATCH 10/10] Make the timeout report survive, and budget the waiting as well as the work The redaction added last round would have crashed instead of redacting. TimeoutExpired.output is bytes even when Popen ran with text=True -- its chunks are buffered before the newline translation that would have decoded them -- while communicate() hands back str, so concatenating the two raises TypeError. The generic handler would then have caught it and written "the canary crashed", throwing away the redacted diagnostics the fix existed to produce. A fix that only worked when it was not needed. Reproduced directly, and the regression test asserts the message says the phase timed out and not that the canary crashed. The budget also counted only the work, never the waiting. Sign-in, the starter download, both token-minting calls and the build-list polling all sit outside the three phases, and at a two-minute-per-request ceiling a slow but living endpoint could add twelve minutes to a sixty-minute budget under a seventy- minute job -- killed before writing a report, so latency would have been reported as an outage. Requests are bounded at a minute each now, the ten minutes they can collectively spend is named and included in the sum, and the job allows eighty-five. The budget test counts the HTTP allowance too, so the next phase added outside the arithmetic fails here rather than in the dark. Verified against production again, since this changed the timeout on every HTTP call the canary makes: build 85f52df6 succeeded in 299 seconds. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/starter-canary.yml | 11 +++---- scripts/ci/starter-canary/starter_canary.py | 26 ++++++++++++++-- .../ci/starter-canary/test_starter_canary.py | 30 +++++++++++++++++-- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/.github/workflows/starter-canary.yml b/.github/workflows/starter-canary.yml index eda99ea2ed5..59f50241109 100644 --- a/.github/workflows/starter-canary.yml +++ b/.github/workflows/starter-canary.yml @@ -72,11 +72,12 @@ jobs: matrix: os: [ubuntu-latest, windows-latest] runs-on: ${{ matrix.os }} - # Must exceed the canary's own budgets (LAUNCH_TIMEOUT 30m + POLL_TIMEOUT - # 20m in starter_canary.py) plus checkout/python/JDK setup. Cut it below - # their sum and the runner kills the job before the canary can write its - # report, turning a slow-but-healthy build into a reported outage. - timeout-minutes: 70 + # Must exceed everything starter_canary.py can spend: HTTP_ALLOWANCE 10m + + # SEED_TIMEOUT 10m + LAUNCH_TIMEOUT 30m + POLL_TIMEOUT 20m = 70m, plus + # checkout/python/JDK setup. Cut it below that and the runner kills the job + # before the canary can write its report, turning slow-but-healthy into a + # reported outage. A test asserts the relationship. + timeout-minutes: 85 steps: - uses: actions/checkout@v6 diff --git a/scripts/ci/starter-canary/starter_canary.py b/scripts/ci/starter-canary/starter_canary.py index 19a945cbd12..0ee0f931461 100755 --- a/scripts/ci/starter-canary/starter_canary.py +++ b/scripts/ci/starter-canary/starter_canary.py @@ -64,6 +64,12 @@ # plus checkout/python/JDK setup. If the runner kills the job first, the canary # never writes its report and the alert job reports an outage that did not # happen -- so these two numbers and that one are a single decision. +HTTP_TIMEOUT = 60 # per request; the console answers in well under a second +# Sign-in, starter download, two token-minting calls and the build-list polling +# all happen outside the three phases below. Budgeting only the phases let a +# slow-but-alive endpoint push the run past the job timeout, which kills it +# before it can write a report -- reporting an outage that was really latency. +HTTP_ALLOWANCE = 600 # 10 min for every request outside the phases SEED_TIMEOUT = 600 # 10 min: resolving and running one small goal LAUNCH_TIMEOUT = 1800 # 30 min: mvnw downloads Maven and the toolchain POLL_TIMEOUT = 1200 # 20 min: waiting for the cloud build to finish @@ -95,7 +101,7 @@ def fetch(opener, url, data=None, headers=None): request = urllib.request.Request(url, data=data, headers=headers or {}) request.add_header("User-Agent", "cn1-starter-canary") try: - with opener.open(request, timeout=120) as response: + with opener.open(request, timeout=HTTP_TIMEOUT) as response: return response.status, response.read(), response.headers, response.url except urllib.error.HTTPError as error: return error.code, error.read(), error.headers, url @@ -331,6 +337,22 @@ def child_environment(): return env +def as_text(value): + """TimeoutExpired.output is bytes even when Popen ran with text=True. + + Its buffered chunks are collected before the newline translation that would + have decoded them, while communicate() hands back str -- so concatenating + the two raises TypeError, and the redacted timeout report this exists to + produce would be replaced by a generic crash report with the diagnostics + thrown away. + """ + if value is None: + return "" + if isinstance(value, bytes): + return value.decode("utf-8", "replace") + return value + + def redact(text, secrets): for value in secrets: if value: @@ -381,7 +403,7 @@ def run(command, cwd, what, timeout, secrets=(), check=True): # and this text is written to the report and copied into a public issue. raise CanaryFailure( f"{what} did not finish within {timeout}s and was terminated.\n\n" - + tail(redact((expired.output or "") + (partial or ""), secrets)) + + tail(redact(as_text(expired.output) + as_text(partial), secrets)) ) from None output = redact(output or "", secrets) if check and process.returncode != 0: diff --git a/scripts/ci/starter-canary/test_starter_canary.py b/scripts/ci/starter-canary/test_starter_canary.py index 77cca4b3256..8a782b88949 100644 --- a/scripts/ci/starter-canary/test_starter_canary.py +++ b/scripts/ci/starter-canary/test_starter_canary.py @@ -272,6 +272,24 @@ def test_timeout_message_hides_the_credentials_in_the_command(self): self.assertNotIn(secret_email, message) self.assertIn("did not finish within", message) + def test_timeout_output_survives_the_bytes_str_split(self): + """TimeoutExpired.output is bytes even under text=True; concatenating raises.""" + secret = "credential-in-the-output" + with self.assertRaises(canary.CanaryFailure) as caught: + canary.run( + [sys.executable, "-c", + f"import time,sys;print('{secret}');sys.stdout.flush();time.sleep(30)"], + cwd=".", what="probe", timeout=1, secrets=(secret,)) + message = str(caught.exception) + self.assertIn("did not finish within", message) + self.assertNotIn(secret, message) + self.assertNotIn("crashed before it could finish", message) + + def test_as_text_normalises_both_shapes(self): + self.assertEqual(canary.as_text(b"bytes"), "bytes") + self.assertEqual(canary.as_text("str"), "str") + self.assertEqual(canary.as_text(None), "") + def test_timeout_kills_the_whole_process_tree(self): """A surviving grandchild could submit a build after this leg failed.""" marker = Path(tempfile.mkdtemp()) / "grandchild-survived" @@ -348,14 +366,20 @@ def test_ordinary_cheap_targets_still_pass(self): class Budgets(unittest.TestCase): - JOB_TIMEOUT_MINUTES = 70 # starter-canary.yml + JOB_TIMEOUT_MINUTES = 85 # starter-canary.yml def test_every_phase_fits_the_job_timeout(self): - """Each blocking phase must be counted, or the runner kills the job first.""" - total = (canary.SEED_TIMEOUT + canary.LAUNCH_TIMEOUT + canary.POLL_TIMEOUT) / 60 + """Every blocking phase counts, HTTP included, or the runner kills the job.""" + total = (canary.HTTP_ALLOWANCE + canary.SEED_TIMEOUT + + canary.LAUNCH_TIMEOUT + canary.POLL_TIMEOUT) / 60 self.assertLess(total, self.JOB_TIMEOUT_MINUTES, "raise timeout-minutes in starter-canary.yml to cover this") + def test_http_requests_are_individually_bounded(self): + """A hung endpoint must not sit inside an otherwise-bounded phase.""" + self.assertLessEqual(canary.HTTP_TIMEOUT, 120) + self.assertLess(canary.HTTP_TIMEOUT, canary.HTTP_ALLOWANCE) + def test_run_requires_an_explicit_timeout(self): """No unbounded default: an uncounted phase is how the budget drifted.""" import inspect