Add ts dev lint domains and ts dev install-hooks to block non-allowlisted URL hosts

[aram356]

Closes #1160

Summary

• Adds ts dev lint domains — a pure-Rust source/config/docs linter that flags non-allowlisted URL hosts in four modes (--staged, --changed-vs <ref>, full-repo, explicit paths). All git operations go through gitoxide (no shelling out to git). Captured URL authorities are canonicalised through the url crate (percent-decoding, IDNA, case, port) before the allowlist check, so lookalike authorities cannot pass on a prefix match.
• Adds ts dev install-hooks — writes a managed pre-commit hook that runs ts dev lint domains --staged into git's own .git/hooks directory (the main repository's from a linked worktree). It never edits git configuration or the working tree; it refuses when core.hooksPath is set in any scope, refuses to clobber an unmanaged hook, and with --force backs the old hook up untouched.
• Refactors the existing ts dev leaf into a subcommand group; lint and install-hooks sit next to proxy.

Design

• Spec: docs/superpowers/specs/2026-05-18-check-domains-design.md
• Plan: docs/superpowers/plans/2026-05-18-ts-dev-lint-domains.md

Allowlists: EXACT_HOSTS, SUBDOMAIN_HOSTS, REFERENCE_HOSTS, plus RFC 2606 reserved TLDs. Suppression marker: // allow-domain: host (and #, <!--, * comment forms). Scanned extensions cover Rust, TS/JS, configs, Markdown, CSS, HTML, env files, and Dockerfiles, matched case-insensitively. Only the linter's own source, E2E suite, spec and plan are exempt. Exit codes: 0 clean, 1 violations, 2 environment error.

Evasion and bypass hardening

Review found four ways the tool could be defeated while reporting success. All are fixed, each with a regression test:

	Issue	Fix
P1	The hook read <git dir>/index, but git supplies a temporary index via GIT_INDEX_FILE for git commit -a and git commit -- <path>, so both forms committed violations that git add + git commit rejected	Honour GIT_INDEX_FILE, falling back to repo.index_path()
P1	Host capture stopped early, missing //github.com%2eevil%2ecom, https://github.com­.com (an invisible soft hyphen) and https://%65vil.com — the first two reported an allowlisted prefix, the third nothing at all	Capture the authority up to a real delimiter, then canonicalise; the protocol-relative pattern no longer ends in a suffix anchor that backtracks into an allowlisted prefix
P2	The JSON report echoed credentials from URLs with escaped soliduses (https:\/\/user:pw@host)	Redactor accepts escaped soliduses
P2	An empty core.hooksPath was treated as unset, so installation reported success while git never read .git/hooks	An empty value is refused like any other override

The wider authority capture is the change most likely to cause false positives, so it was checked against the whole repository: a full-repo audit reports exactly the same 484 violations as the pre-change build, byte-identical — no false positives, no lost detections.

Test plan

• cargo fmt --all -- --check
• cargo clippy --package trusted-server-cli --target <host> --all-targets --all-features -- -D warnings
• cargo clippy-fastly, cargo clippy-axum
• cargo test --package trusted-server-cli --target <host> — 351 tests across 8 suites, including real-git end-to-end tests for the installer (branch-planted .githooks/post-checkout stays inert, linked worktree shares the hook, global and empty core.hooksPath refused, temporary-index commit forms blocked)
• Each bypass regression test confirmed to fail against the unfixed code
• ts dev lint domains --changed-vs origin/main on this branch: clean
• ts dev lint domains full audit: diagnostic only, reports the repo's pre-existing violations (no CI gate in v1)
• cd docs && npm run format

[ChristianPavilonis]

Automated Review:

Summary

Reviewed the domain-lint and hook-installation changes. I found one confirmed self-exclusion defect, submitted inline.

Findings by priority

• P1: 1 inline finding.

CI

All currently reported PR checks are passing. git diff --check is clean.

Existing Reviews

No existing submitted reviews were returned when checked; this review does not duplicate prior feedback.

[prk-Jr]

Summary

Adds ts dev lint domains (four scan modes, gitoxide-only) and ts dev install-hooks. The pure-function layer — host extraction, allowlist matching, suppression markers — is well factored and unusually well tested, including userinfo-bypass and rename regressions. Three defects block: the self-exclusion constant points at a path that no longer exists, so the linter flags its own source and this PR fails its own --changed-vs gate; install-hooks silently disables any hooks already in .git/hooks; and both commands error out when run from a subdirectory.

Verified against the PR head in an isolated worktree with a binary built from it (aarch64-apple-darwin): cargo fmt --all -- --check clean, cargo clippy -p trusted-server-cli --all-targets -- -D warnings clean, cargo test -p trusted-server-cli 236 + 28 + 4 + 29 + 1 pass. Evidence for each defect is in the inline comments.

1 of the inline comments below carries a one-click GitHub suggestion — use Commit suggestion to apply it as a commit on the PR branch. The remaining comments describe the fix in prose because the change spans several files, touches Cargo.lock, or needs a new helper, and can't be auto-applied.

Blocking

🔧 wrench

• SELF_PATH points at a path that does not exist — self-exclusion is dead — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:883
• install-hooks silently disables every hook already in .git/hooks — see inline at crates/trusted-server-cli/src/commands/dev/install_hooks.rs:229
• Both commands fail from any subdirectory (gix::open does not discover the repo root) — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:995

❓ question

• Stage-2 dogfooding policy for the linter's own test file and the two design docs — see the cross-cutting section below.

Non-blocking

♻️ refactor / 🤔 thinking / 🌱 seedling / ⛏ nitpick

• Violation report prints the stale src/dev/lint/domains.rs hint path (suggestion) — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:2054
• temp-env dev-dependency added but never used — see inline at crates/trusted-server-cli/Cargo.toml:81
• Four new deps bypass [workspace.dependencies] — see inline at crates/trusted-server-cli/Cargo.toml:41
• Absolute-URL regex accepts single-label hosts; protocol-relative one does not — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:341
• test_support.rs and tests/common/mod.rs are byte-identical, kept in sync by comment — see inline at crates/trusted-server-cli/tests/common/mod.rs:8
• Diff modes don't skip binary blobs, full-repo mode does — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:950
• --staged writes a tree object into the object database on every run — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:1025

Cross-cutting / body-level findings

• ❓ Stage-2 dogfooding policy. After the SELF_PATH fix, ts dev lint domains --changed-vs origin/main on this head still reports 91 violations across 3 of this PR's own new files: crates/trusted-server-cli/tests/lint_domains_cli.rs (30), docs/superpowers/specs/2026-05-18-check-domains-design.md (34), docs/superpowers/plans/2026-05-18-ts-dev-lint-domains.md (27) — all from the deliberate test.com / evil.com / cdn.example.evil fixtures the spec's own test cases require. What is the intended resolution: extend the exclusion list to the linter's test file and docs/superpowers/**, sprinkle allow-domain: markers, or accept that the Stage 2 changed-lines gate cannot be turned on until the Stage 1 cleanup covers these? Worth answering in this PR, because it decides whether the exclusion policy needs to grow before the gate exists.

• 📌 Plan Phase 8 (documentation) did not ship. The plan's Task 8.1 (CONTRIBUTING.md — "Pre-commit URL-host linter" install steps) and Task 8.2 (README.md mention) have no counterpart in the diff, so nothing in the repo tells a contributor that ts dev install-hooks exists or that a pre-commit linter is expected. Either land those two doc edits here or open a follow-up so the install step isn't discoverable only from the design doc.

• 📝 PR description's exit-code contract overstates the implementation. The description lists "130 cancelled", but there is no CliError::Cancelled variant and no 130 mapping — finish() in commands/dev/mod.rs maps ViolationsFound → 1, EnvironmentError → 2, everything else → 1. The spec leaves the 130 model explicitly undecided ("Pick one model"), so the code is self-consistent; only the description is ahead of it.

• 👍 Genuinely strong adversarial testing. The userinfo-bypass regressions (https://github.com@test.com/path → test.com, https://a@b@c.evil → c.evil) close a real allowlist bypass; the pure-rename and rename+edit cases pin a bug that a naive path-map walk would reintroduce; and works_without_git_on_path (env_clear + empty PATH) actually proves the no-subprocess claim rather than asserting it. The suppression-marker bypass tests (allow-domain inside a URL path, host literally named allow-domain) are the kind of tests reviewers usually have to ask for.

CI Status

• integration tests (Fastly EC lifecycle): PASS
• integration tests: PASS
• browser integration tests: PASS
• CodeQL: PASS
• Analyze (actions): PASS
• Analyze (rust): PASS
• Analyze (javascript-typescript): PASS
• prepare integration artifacts: PASS
• vitest: PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo test: PASS (required)
• cargo test (ts CLI, native): PASS
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• format-typescript: PASS (required)
• format-docs: PASS (required)
• cargo fmt: PASS (required)

[aram356]

Review addressed — a05cf2a

Thanks for the thorough pass. All three blocking findings and the non-blocking cleanups are resolved; each inline thread has a reply. Summary of the cross-cutting / body-level items:

Stage-2 dogfooding policy (❓). Resolved by extending the self-exclusion rather than sprinkling markers. After the SELF_PATH fix, the remaining 91 violations were all deliberate fixtures in the linter's own E2E test file and its two design docs, so path_is_scanned now also skips crates/trusted-server-cli/tests/lint_domains_cli.rs and the docs/superpowers/ tree (internal design prose that quotes test.com/evil.com as examples of what the linter catches). Result: ts dev lint domains --changed-vs origin/main on this head reports 0 — the Stage-2 changed-lines gate can be turned on without a separate Stage-1 cleanup.

Plan Phase 8 docs (📌). Landed here: CONTRIBUTING.md gains a "Pre-commit URL-host linter" section with the ts dev install-hooks / ts dev lint domains steps, and README.md now points contributors at it.

PR description exit-code (📝). Corrected — the description no longer claims "130 cancelled"; it now matches finish() (0 clean / 1 violations / 2 environment error). The 130 model stays explicitly undecided in the spec.

Verified on aarch64-apple-darwin: cargo fmt --all -- --check clean, cargo clippy -p trusted-server-cli --all-targets -- -D warnings clean, cargo test -p trusted-server-cli green (239 lib + 28 + 29 + 1 + 4), docs prettier clean, and the self-scan / subdirectory / changed-vs behaviors checked against a binary built from this head.

[aram356]

Round 4 feedback addressed in 944df4d — both findings from @prk-Jr. Each was reproduced against a built CLI before any change and re-verified against the same reproduction afterwards; replies are on the individual threads.

Host extraction (; , ( ) ') — all are accepted inside a hostname by the WHATWG parser, so terminating the capture on them handed the allowlist an example.com prefix.

The review warned that deleting delimiters would trade the bypass for Markdown and source-punctuation false positives. I measured rather than guessed: removing every soft delimiter added 115 new violations across a full-repo scan (almost all format!("https://{host}{path}")), and narrowing to just these five still left 33. So the split is by position, not by character:

• { } [ ] stay terminators — braces enclose interpolation, and the URL parser rejects brackets outside the IPv6 branch the regex captures separately.
• The rest are admitted into the capture and handled after it: everything from a ' onwards is cut (a quote inside an authority marks where the source string closed and the rest is code), then a trailing $#*(),;!&= run is trimmed, extending the previous round's interpolation trim.

Only the leading label and trailing run are affected, so embedded occurrences still reach the allowlist — tests assert example.com;unapproved.internal, example.com(unapproved.internal and github.com$unapproved.internal are each reported whole, so the trims cannot become a new bypass.

IDNA TLDs — applied the suggestion. Canonicalisation maps a Unicode TLD to punycode, whose xn-- prefix carries hyphens and digits, so the ASCII-alphabetic-only test discarded it. A test asserts the protocol-relative result equals the absolute form so the two paths cannot drift again.

Effect on the repo: net zero. The violation count is unchanged at 486, with two lines now reporting the more accurate origin.(example).com where the old class truncated to origin.

Also in this push: merged main, which had rewritten CONTRIBUTING.md from 174 lines to 61 — I took that rewrite wholesale and re-added the pre-commit-linter documentation condensed into its new Verify section. The PR is MERGEABLE again.

Verification — every CI gate run locally, all green: cargo fmt --all --check; all eight clippy gates including the new clippy-cli / clippy-codegen; all four adapter suites; parity; the CLI suite — 3876 tests, 0 failures; plus 1130 vitest, JS and docs format. Coverage added for both findings in unit tests and in staged mode (the pre-commit path), plus a multi-byte-host test since the quote cut slices by byte index and IDNA makes that reachable.

[prk-Jr]

One blocking correctness issue: consecutive single-quoted URLs can cause the linter to silently skip a disallowed host, including in staged mode.

Validation: reproduced against the reviewed commit; the suggested one-line replacement makes the reproduction report the violation and exit 1. All 104 linter unit tests and 38 CLI integration tests pass with the replacement. Scratch changes were restored. All 20 GitHub checks passed when inspected during review.

[aram356]

Round 5 feedback addressed in fcba41d — the single blocking finding from @prk-Jr, applying the suggested one-character change.

The bug was mine from round 4. I had removed ' from the terminator class because it is legal inside a WHATWG hostname, trimming a trailing one after capture instead. That was wrong in a way the trim could not repair: a match that does not stop at the quote runs past the end of its own string and consumes the next URL's scheme, and captures_iter cannot revisit consumed input.

Reproduced before changing anything, and it was slightly worse than reported — a list of two disallowed hosts reported only one:

['https://unapproved.internal','https://other.internal']  ->  ['unapproved.internal']

Both are reported now. A silent false negative is the one outcome this linter must not have.

Cost of the fix: zero. Since ' was removed last round specifically to avoid false positives, I measured before accepting: a full-repo scan with only this character changed gives 486 violations before and after, no additions, no removals. The trailing-quote shapes that motivated the trim (expect(new URL('https://host').href)) are now handled by the terminator, which is strictly better — it bounds the match instead of repairing it afterwards.

Dead code removed. With the quote terminating the capture, the trim's leading find('\'') cut became unreachable. I verified that by asserting on it across a full-repo scan and all unit tests before deleting, rather than assuming, and renamed the multi-byte test that referenced it.

Verification — all gates green locally: cargo fmt --all --check; all eight clippy gates; all four adapter suites; parity; CLI suite — 3878 tests, 0 failures; plus 1130 vitest and JS/docs format. Regression coverage for consecutive single-quoted URLs (three-host lists, double quotes, mixed quoting) and a staged-mode CLI test. Re-confirmed the earlier rounds' reproductions still pass. Spec records why the quote terminates despite being host-legal, so this does not get re-removed in a future round.