Gate PRs on non-allowlisted URL hosts in changed lines

[dhruv8sh]

Summary

• Adds a lint-domains CI job that runs ts dev lint domains --changed-vs HEAD^1 on every pull request. Today the pre-commit hook is the only enforcement, and it is skipped by git commit --no-verify, by clones that never ran ts dev install-hooks, and by commits made in the GitHub web UI. An unreviewed outbound host is a supply-chain concern for an edge service, so the check has to run server-side.
• This is Stage 2 of the check-domains design. Draft until Add ts dev lint domains and ts dev install-hooks to block non-allowlisted URL hosts #733 merges: the job calls a subcommand that only exists there, so it fails on this PR until then. Per Add a CI gate for non-allowlisted URL hosts on changed lines #1215, it should also land after the cleanup in Clean up pre-existing non-allowlisted URL hosts so the full-repo audit can gate #1216.

Changes

File	Change
.github/workflows/test.yml	New lint-domains job (ubuntu, pull_request only): depth-2 checkout, pinned Rust toolchain with its own cache key, then cargo run … --bin ts -- dev lint domains --changed-vs HEAD^1 for the host target
AGENTS.md	CI gate 9, including the equivalent local command (--changed-vs origin/main)

Departures from the issue

• Own job, not a step in test-cli. It gets a separate check name that branch protection can require, needs no matrix special-casing, and runs in parallel instead of after the slow NSS and browser tests. The cost is one more toolchain setup. It uses its own cache key so it can't race test-cli into saving a cache without the test builds.
• --changed-vs HEAD^1 with fetch-depth: 2, not $GITHUB_BASE_REF with fetch-depth: 0. A PR checkout is GitHub's merge commit, so its first parent is the exact base commit GitHub merged against. Depth 2 fetches that parent without cloning the full history (~295 MiB) on every PR. Depth 1 does not work: the parent isn't present, so the linter exits 2 on every PR. It fails closed, never open.

Note for #733's rebase

Besides textual conflicts in crates/trusted-server-cli/Cargo.toml, commands/dev/mod.rs and lib.rs, #733 conflicts with current main in a way that breaks the build: it gates output::info/output::warn to target_os = "macos", but main now builds the dev proxy on Linux too, and the proxy calls them. Widening the gate to any(target_os = "macos", target_os = "linux") fixed it in a local merge.

Closes

Part of #1215. Two done-when items are left for after merge: a real PR shown failing the gate on GitHub (needs #733 on main), and the Stage 3 decision (clean up and gate the full audit, or use a baseline file).

Test plan

• cargo test-fastly && cargo test-axum: not run, no Rust changes. The base commit 7a0ecb4c is green on main
• cargo clippy-fastly && cargo clippy-axum: not run, same reason
• cargo fmt --all -- --check
• JS tests: no JS changes
• JS format: no JS changes
• Docs format: cd docs && npm run format
• WASM build: no Rust changes
• Manual testing via fastly compute serve: not applicable
• Other:
  • Markdown format outside docs/ (CI gate 8) passes.
  • Simulated the Actions PR checkout locally (bare upstream repo, a refs/pull/N/merge merge commit, a fetch at the given depth, ts built from Add ts dev lint domains and ts dev install-hooks to block non-allowlisted URL hosts #733):
    • Depth 2 with HEAD^1: a PR adding https://unapproved.internal/… exits 1 and flags the line; a PR adding only an example.com URL exits 0.
    • Depth 1 exits 2 with both HEAD^1 and main as the base.
  • In a local merge of main + Add ts dev lint domains and ts dev install-hooks to block non-allowlisted URL hosts #733 with this change on top, the Linux CLI steps (clippy, cargo test, real-NSS trust tests) pass. The gate exits 0 on this change and 1 on a follow-up commit adding a disallowed host.

Checklist

• Changes follow AGENTS.md conventions
• No unwrap() in production code: no Rust changes
• Uses tracing macros (not println!): no Rust changes
• New code has tests: CI-only change, verified by the local simulation above
• No secrets or credentials committed

[dhruv8sh]

PR CI will fail until #733 is merged.

[dhruv8sh]

Important

Action needed after merge: @aram356

Once this PR merges, please add this check to the required status checks in the main ruleset:

ts dev lint domains (changed lines)

Until it is required, a failing red check is shown but does not block merging.

[prk-Jr]

Summary

Adds a PR-only lint-domains job that runs ts dev lint domains --changed-vs HEAD^1, plus the matching AGENTS.md gate. The diff-base choice is correct: the linter scans against merge-base(<ref>, HEAD), and on GitHub's merge-commit checkout HEAD^1 is the base tip, so exactly the PR's added lines are scanned. Nothing blocking.

1 of the inline comments below carries a one-click GitHub suggestion — use Commit suggestion to apply it.

Non-blocking

♻️ refactor

• Cache key is never reusable across PRs — see inline at .github/workflows/test.yml:319

👍 praise

• Depth-2 / HEAD^1 rationale, fail-closed at depth 1 — see inline at .github/workflows/test.yml:304

Cross-cutting / body-level findings

• 📌 Gate is advisory until branch protection requires it — ts dev lint domains (changed lines) isn't in the required-checks list (currently format-docs, cargo fmt, cargo test, format-typescript), so a PR that adds a disallowed host can still merge with this check red. Worth tracking as an explicit post-merge step for a repo admin.
• 🌱 The PR under test can relax its own gate — the linter is compiled from the PR's own source (the allowlist lives in is_allowed, crates/trusted-server-cli/src/commands/dev/lint/domains.rs:239) and the workflow file comes from the PR too, so a single diff can widen the allowlist or drop the job. Both are visible in review, so this is fine for now; the repo has no CODEOWNERS, and owners on crates/trusted-server-cli/src/commands/dev/lint/ and .github/workflows/ would make such edits need a specific approval.

CI Status

All checks pass, including the new ts dev lint domains (changed lines).

• cargo fmt: PASS (required)
• cargo test: PASS (required)
• format-docs: PASS (required)
• format-typescript: PASS (required)
• ts dev lint domains (changed lines): PASS
• cargo test (ts CLI, native) (ubuntu-latest / macos-latest): PASS
• cargo test (axum native), cargo test (cross-adapter parity), cargo check (cloudflare native + wasm32-unknown-unknown), cargo check/build/test (spin native + wasm32-wasip1): PASS
• integration tests, integration tests (Fastly EC lifecycle), browser integration tests, prepare integration artifacts: PASS
• vitest, CLAUDE.md symlink guard: PASS
• CodeQL, Analyze (rust / actions / javascript-typescript / python): PASS

[aram356]

Summary

Adds a PR-only lint-domains job that runs ts dev lint domains --changed-vs HEAD^1 on a depth-2 checkout, plus the matching AGENTS.md gate. In a replay of the Actions merge-ref checkout, HEAD^1 behaves as described: a clean PR exits 0 and is not blamed for a host main added after the fork, a new host exits 1, and depth 1 exits 2. The cargo-cli cache key matches main's test-cli caches (env hash 80d47658), and after a test-cli-shaped build cargo run --bin ts rebuilds 22 of ~430 crates. What blocks: the branch conflicts with main, so CI hasn't run this head; the job builds the TSJS bundles with the runner's unpinned Node; and there's an open question on when the check should become required.

2 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch for several at once) to apply them as commits on the PR branch. The remaining comments describe the fix in prose because the change touches multiple files or lines outside the diff and can't be auto-applied.

Blocking

🔧 wrench

• Merge conflict with main; CI hasn't run at this head — see inline at AGENTS.md:365
• Pin Node before building ts — see inline at .github/workflows/test.yml:325

❓ question

• When should this check become required? — see Cross-cutting below

Non-blocking

♻️ refactor

• Add --verbose so a green run shows what was scanned — see inline at .github/workflows/test.yml:330

📌 out of scope

• Cache key stops matching test-cli once #1259 lands — see inline at .github/workflows/test.yml:322

🏕 camp site

• Document the gate in CONTRIBUTING.md and /check-ci — see Cross-cutting below

Cross-cutting / body-level findings

• ❓ When should this check become required? The description says this should land after #1216, which is still open, and the follow-up comment asks for the check to be required right after merge. I replayed the gate (the linter built from this PR's head, --changed-vs C^1 with HEAD at C) over the last 80 first-parent commits on main: 24 would have failed. Of the 13 PRs merged since #733, 2 would have failed, and neither added an outbound host:

  • #1213: http://127.0.0.1:$PORT/health in docs/superpowers/plans/2026-09-25-git-version-header.md:918 is reported as host 127.0.0.1:$port.
  • #1188: a moved test line that already contained https://test-publisher.com/auction (crates/trusted-server-core/src/auction/endpoints.rs:838).

  Across all 24, the false positives fall into three classes: documentation address ranges (192.0.2.1, 2001:db8::7), loopback with a shell-variable port (127.0.0.1:$PORT, localhost:$TS_PORT), and placeholders (https://host#f in a comment, format!("https://sync.{source_domain}/pull") reported as sync). The rest are real under the current policy: the APS hosts, test.com and test-prebid.com in tests, and links to AWS docs and RFCs. Can the check stay non-required until a linter issue for those three classes is fixed and #1216 lands? Or should this PR go back to draft until then, as the description says?

• 🏕 Document the gate in CONTRIBUTING.md and /check-ci. CONTRIBUTING.md's "Pre-commit URL-host linter" section still presents the hook as the only check, and .claude/commands/check-ci.md ("Run all CI checks locally, in order") doesn't run it. Neither file is in this diff, so this can't be a suggestion. Proposed addition after the "To scan on demand" paragraph (passes the gate-8 Prettier check):

  CI runs the same check on every pull request: the `lint-domains` job scans the
  lines the PR adds against its base branch, so `--no-verify` or a missing hook
  does not let a host through. Editing or moving a line that already contains a
  non-allowlisted host counts as adding it. Reproduce the job locally with
  `ts dev lint domains --changed-vs origin/main`.

  Also end the suppression sentence with in a comment (<!-- allow-domain: <host> --> in Markdown). The HTML-comment form does suppress a Markdown line. For check-ci.md, add a step 8 that runs cargo run --manifest-path crates/trusted-server-cli/Cargo.toml --target "$(rustc -vV | sed -n 's/host: //p')" --bin ts -- dev lint domains --changed-vs origin/main.

CI Status

• Analyze (javascript-typescript): PASS
• Analyze (python): PASS
• Run Tests, Format, Integration Tests, and CodeQL (actions, rust) workflows: not run at 7fecdbdd1. GitHub builds no merge ref while the PR conflicts with main. The last full run, at ab4c378c8, passed all 22 checks, including ts dev lint domains (changed lines).

[aram356]

🔧 wrench — This conflicts with main. #1226 already took gate 10 for the wasm clock lint, so GitHub builds no merge ref and the Run Tests workflow hasn't run at 7fecdbdd1: only the two CodeQL checks reported. The cargo-cli cache change from the last round has therefore never run in CI.

Merge main, keep both lines, and renumber this one:

10. Wasm clock lint: `scripts/lint-wasm-clock.sh` (crates built into the Cloudflare Worker must use `web_time`, not the `std::time::{Instant,SystemTime}` `now`/`elapsed` methods, which panic there)
11. URL-host linter on lines the PR adds (`lint-domains` job, PRs only); run it locally with `cargo run --manifest-path crates/trusted-server-cli/Cargo.toml --target <host-triple> --bin ts -- dev lint domains --changed-vs origin/main`

The resolved file passes the gate-8 Prettier check. Once CI runs, the lint-domains log should show a restore from a v0-rust-cargo-cli-Linux-Linux-x64-80d47658-… key; main holds several under that prefix.

Apply manually: a suggestion can't resolve a merge conflict.

[aram356]

🔧 wrench — Building ts compiles trusted-server-core, and trusted-server-js's build script runs npm ci and npm run build with whatever npm is on PATH. This job never sets up Node, so the green run at ab4c378c8 built the TSJS bundles with the runner image's Node (tsjs: Building per-module bundles in the log). That's the #1204 problem, and #1257's guard rejects it: on a merge of #1257 and this PR, scripts/check-ci-node-pin.py exits 1 with job lint-domains ... compiles Rust in step Lint URL hosts on lines added by this PR before an unconditional actions/setup-node step pinned to .tool-versions. With this suggestion applied it reports ok: 21 jobs in 6 workflows pin Node before compiling Rust.

Suggested change

	# The `ts` CLI is host-target only; the workspace default is wasm32-wasip1.
	# Building `ts` compiles trusted-server-core, whose trusted-server-js
	# build script runs the TSJS build with the `npm` on PATH, so pin Node
	# first (see #1204).
	- name: Use Node.js for the TSJS build
	uses: actions/setup-node@v4
	with:
	node-version-file: .tool-versions
	# The `ts` CLI is host-target only; the workspace default is wasm32-wasip1.

(Checked with actionlint (no new findings) and #1257's guard on the merged tree. setup-node reads the nodejs 24.12.0 line from .tool-versions.)

[aram356]

♻️ refactor — On success the linter prints nothing. The green run at ab4c378c8 goes straight from Running target/x86_64-unknown-linux-gnu/debug/ts dev lint domains --changed-vs 'HEAD^1' to post-job cleanup, so the log can't show whether any lines were scanned. --verbose writes scanned N lines in <path> per file to stderr and doesn't change the exit code, which makes a vacuous pass visible.

Suggested change

	dev lint domains --changed-vs HEAD^1
	dev lint domains --changed-vs HEAD^1 --verbose

(In a depth-2 replay of the Actions merge-ref checkout, a clean PR printed scanned 1 lines in c.md and exited 0.)

[aram356]

📌 out of scope — This key only matches test-cli's while both jobs hash the same installed toolchains. rust-cache puts every rustup toolchain list entry into the key; this job's log lists 1.95.0 and the image's 1.99.0 stable. #1259 removes the image's stable in test-cli through the new ./.github/actions/setup-rust, so once it lands the hashes differ and this job silently goes back to cold builds (2m31s for the build step on the last uncached run). Whichever of #1254 and #1259 merges second should move this job to the composite action and drop its Retrieve Rust version step:

      - name: Set up Rust toolchain
        uses: ./.github/actions/setup-rust
        with:
          cache-key: cargo-cli
          save-cache: "false"

[ChristianPavilonis]

Approved. The dedicated lint-domains CI job adds server-side enforcement for URL hosts in changed PR lines, with the local equivalent documented in AGENTS.md.