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. It lands as a non-blocking check and stays non-required until the linter false positives in Stop ts dev lint domains reporting false-positive hosts #1417 are fixed and the cleanup in Clean up pre-existing non-allowlisted URL hosts so the full-repo audit can gate #1216 lands.

Changes

File	Change
.github/workflows/test.yml	New lint-domains job (ubuntu, pull_request only): depth-2 checkout, pinned Rust toolchain restoring test-cli's cache read-only, pinned Node for the TSJS build, then cargo run … --bin ts -- dev lint domains --changed-vs HEAD^1 --verbose for the host target
AGENTS.md	CI gate 11, including the equivalent local command (--changed-vs origin/main, or the PR's base branch when it targets another branch)
CONTRIBUTING.md	The CI job, the base-branch caveat, and the Markdown <!-- allow-domain: <host> --> suppression form
.claude/commands/check-ci.md	Step 8 runs the gate against origin/main
docs/superpowers/specs/2026-05-18-check-domains-design.md	Stage 2 records the job design: own job, HEAD^1, depth 2, fails closed

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 restores test-cli's cache read-only and never saves, 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.

Closes

Part of #1215. The "real PR shown failing the gate" done-when item is met by the run on #1228. Left for after merge: the Stage 3 decision (clean up and gate the full audit, or use a baseline file).

Spec

Spec: docs/superpowers/specs/2026-05-18-check-domains-design.md (Migration to CI, Stage 2)

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, including the round-3 doc changes.
  • 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]

Note

Withdrawn. This check stays non-required for now. It becomes required only after the linter false positives in #1417 are fixed and the cleanup in #1216 lands; a new request will follow then.

Once this PR merges, please add this check to the required status checks in the main ruleset: ts dev lint domains (changed lines)

[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).

[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.

[aram356]

Summary

Round 2 at d0d74cc96. All four inline findings from round 1 are fixed or agreed:

• The AGENTS.md conflict is resolved, with the linter at gate 11.
• Node is pinned: the job log shows Resolved .tool-versions as 24.12.0, and #1257's checker passes on a merge with this branch.
• --verbose works: the log shows scanned 1 lines in AGENTS.md and scanned 45 lines in .github/workflows/test.yml.
• The #1259 follow-up is agreed, and it is enforced: the two branches merge cleanly as text, but #1259's guard in the required cargo fmt job fails on this job's direct setup-rust-toolchain step, so whichever lands second can't miss the migration.

The two body-level findings from round 1 got no reply. The question about when the check becomes required is still open, so I've asked it again inline, with the gate's first real run as evidence.

1 of the inline comments below carries a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch for several at once) to apply it as a commit 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

❓ question

• When does this check become required? — see inline at .github/workflows/test.yml:300

Non-blocking

📝 note

• Gate 11 collides with #1257's gate 11 — see inline at AGENTS.md:367
• The PR description is stale — see Cross-cutting below
• The shared cache missed at this head — see Cross-cutting below

🏕 camp site

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

⛏ nitpick

• The local command assumes main as the base — see inline at AGENTS.md:367

Cross-cutting / body-level findings

• 📝 The PR description is stale. It still says "Draft until #733 merges" (#733 merged on 2026-10-08), describes a "pinned Rust toolchain with its own cache key" and "It uses its own cache key so it can't race test-cli" (the job now restores test-cli's key read-only), and lists the AGENTS.md change as "CI gate 9" (it is gate 11). It doesn't mention the Node or --verbose steps, and "Note for #733's rebase" no longer applies. The "real PR shown failing the gate" done-when item is now met by the run on #1228. main's PR template also gained a Spec: line; docs/superpowers/specs/2026-05-18-check-domains-design.md (Migration to CI, Stage 2) fits it.

• 🏕 Document the gate in CONTRIBUTING.md, /check-ci, and the spec (still open from round 1). CONTRIBUTING.md's "Pre-commit URL-host linter" section still presents the hook as the only check; the round-1 proposed text still applies unchanged after main's new spec section. .claude/commands/check-ci.md still doesn't run the gate; it becomes step 9 if #1257 lands first, since #1257 takes step 8. New this round: the spec's "Migration to CI" Stage 2 still describes --changed-vs $GITHUB_BASE_REF, fetch-depth: 0, and reusing the test-cli lane, which this PR deliberately replaced. A sentence there recording the HEAD^1 / depth-2 / own-job shape keeps the Stage 3 starting point accurate.

• 📝 The shared cache missed at this head. Both this job and test-cli logged No cache found for v0-rust-cargo-cli-Linux-Linux-x64-80d47658, so this was a cold 2m25s build. main currently has no cargo-cli cache at all: its only main-scoped Rust cache is cargo-Linux (427 MiB), while PR-scoped caches fill the 10 GB quota and evict main's. Until #1259 stops PR saves, the first push of each PR builds cold here, and later pushes restore test-cli's save from the same PR. Nothing to change in this PR; the job comment describes the right mechanism.

CI Status

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

The branch is 3 commits behind main (#1385, #1323, #1183); a trial merge is clean.

[dhruv8sh]

Docs follow-up from the review summary, in 36797df. The gate is now documented in three places. CONTRIBUTING.md's "Pre-commit URL-host linter" section has your proposed paragraph (plus base-branch wording) and the <!-- allow-domain: <host> --> Markdown form. /check-ci has a step 8 that runs the gate against origin/main. The spec's "Migration to CI" Stage 2 has a paragraph recording the HEAD^1 / depth-2 / own-job design.