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.

[ChristianPavilonis]

Summary

Reviewed head 026fafcbc8fc6a627e38d309e400d2b47ff34a94 against 125ed242ec6925f2b41f9c84addf75e61b08ae1c. Requesting changes because runtime probes found a branch-controlled hook execution path, URL and rename false negatives in the linter, and installer safety failures not covered by the green CI suite. No repository files were modified during review.

[prk-Jr]

Summary

Reviewed head 026fafcb against merge-base 125ed242 in an isolated worktree, with a binary built from this head used for runtime probing. The linter and installer are carefully built — the userinfo-bypass handling, the unused-suppression warnings, and the honest in-code note about --staged writing a real tree object all show real care, and 117 tests is serious coverage.

This review covers only findings not already raised in the open review on this head; I am not restating those threads. Requesting changes on two of my own: the installer leaves an untracked, un-gitignored artifact carrying a machine-specific absolute path in every contributor's checkout, and the CONTRIBUTING prose this PR adds overstates a safety guarantee that a runtime probe contradicts.

2 of the inline comments below carry a one-click suggestion. Both were applied to a scratch worktree in isolation and cleared cargo fmt --all -- --check, cargo clippy -p trusted-server-cli --target aarch64-apple-darwin --all-targets -- -D warnings, and the full cargo test -p trusted-server-cli suite (262 lib + 5 + 28 + 29 + 1, zero failures), with a byte-exact pre/post-verification patch check showing no drift. The remaining comments describe the fix in prose because it spans multiple files or depends on how another open thread resolves.

No repository files were modified by this review.

Blocking

🔧 wrench

• .githooks/ is untracked and un-gitignored — see inline at crates/trusted-server-cli/src/commands/dev/install_hooks.rs:228
• CONTRIBUTING overstates the installer's refusal guarantee — see inline at CONTRIBUTING.md:132

Non-blocking

♻️ refactor

• resolve_base_ref rejects object ids and revision expressions (suggestion) — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:1191
• set_local_config_value would overwrite an unparsable .git/config (suggestion) — see inline at crates/trusted-server-cli/src/commands/dev/install_hooks.rs:170
• Spec and code disagree on the exclusion list — see inline at docs/superpowers/specs/2026-05-18-check-domains-design.md:527

🤔 thinking / 📝 note / ⛏ nitpick

• Underscore is a further authority bypass — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:341
• --format json echoes the whole source line, and .env* is in scope by design — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:1993
• Extension matching is case-sensitive — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:946
• --staged tree synthesis is not mode-faithful — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:1050
• FileViolation's serde renames invert intuition — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:1987
• write_atomic leaks its temp file on rename failure and never fsyncs — see inline at crates/trusted-server-cli/src/commands/dev/install_hooks.rs:124

Cross-cutting / body-level findings

• 🤔 85 new crates, and git network transport ships inside ts. cargo tree -p trusted-server-cli --target aarch64-apple-darwin -e normal -i gix-transport resolves through gix -> gix-protocol -> gix-transport, so the git wire protocol and transport layer are compiled into the shipped ts binary for a tool that never opens a socket. Credit where due: default-features = false did real work — gix-archive, gix-blame, gix-status and gix-merge are lockfile-only and never built, there are no duplicate versions (the spec's cargo tree claim holds), and zlib-rs is pure Rust with no C or OpenSSL pulled in. Depending on the granular subcrates actually used (gix-discover, gix-index, gix-diff, gix-object, gix-config) instead of the umbrella gix crate would drop the transport surface. CLAUDE.md asks for justification on new dependencies; 85 crates for a dev-workflow linter is worth an explicit line in the spec either way.

• 🤔 Shell scripts are out of scope, and that is where network calls actually live. .sh, Makefile, .jsx, .py and .scss all fall through path_is_scanned. This repo has 10 tracked .sh files, 8 of which contain http(s):// URLs. The spec documents the scanned-extension list explicitly, so this is a deliberate design boundary rather than a bug — but a linter whose stated purpose is catching non-allowlisted egress hosts skipping scripts/*.sh is worth revisiting before Stage 2, or at least recording as a known gap in the spec's threat model.

• 🌱 No ts dev install-hooks --uninstall. Installation is a documented one-liner in both README and CONTRIBUTING; removal requires knowing to unset core.hooksPath and delete .githooks/ by hand. The restore hint only prints when a previous value was displaced, so a contributor installing into a clean checkout is told nothing about how to back out.

• 📝 A concern I raised and then withdrew, recorded so it does not resurface. added_lines runs Diff::compute(Algorithm::Myers, ...) with no size cap, which looked like a pre-commit latency risk on large staged files. Measured on a 9.1 MB / 120,000-line staged .ts file: 2.9 s in an unoptimised debug build. Not a problem, and not a finding.

• 👍 Praise. The userinfo-bypass handling in both regexes, with named regression tests covering user:password@, multiple @, and the protocol-relative form, is the kind of thing that usually gets missed. So is the unused-suppression-marker warning, which stops allow-domain: from silently rotting. The in-code note that write_index_to_tree persists a real tree object into .git/objects documents a real cost honestly instead of hiding it. And resolving the earlier duplicated-fixture thread with #[path] rather than a sync-by-comment copy is the right call.

CI Status

All 19 reported checks PASS at this head. Required checks are marked.

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

[aram356]

Review addressed at 7077b7a

Both rounds are covered; every inline thread has a reply naming its commit. Seven of the installer findings traced to one design choice, so that is the first commit:

• fbbb6ae The hook now lives in <common git dir>/hooks/pre-commit. No .git/config write, no .githooks/ in the working tree, no core.hooksPath change. The effective core.hooksPath from any scope is refused regardless of --force. Symlinked hooks directories are refused; existing hooks are detected with symlink_metadata and backed up unread under --force; the write is atomic with fsync, 0755, and temp cleanup. gix-config is dropped. New real-git E2E tests prove a branch-planted post-checkout stays inert, a linked worktree shares the hook and the hook blocks a commit there, and a global core.hooksPath is refused.
• 4c510b2 Whole-authority capture, \/ unescaping, and url-crate canonicalisation before the allowlist check. All seven authorities from the calibration table are now reported; the audit numbers did not move.
• 76c7b26 Renames from unscanned paths are additions in both diff modes.
• 0c9ba26 Self-exclusion narrowed to four named files, spec list synced, case-insensitive extension and lockfile matching.
• 96e8371 Fixtures pinned to main with an init.defaultBranch=master regression.
• e48dd8b Revspec fallback, userinfo redaction in the JSON excerpt, JSON field names, stage-0 and mode-faithful index tree.
• 7077b7a Spec records the shell-script scope gap as a known v1 boundary.

Body-level items: the 85-crate footprint is now justified in the spec's dependency notes (gix-protocol is a non-optional dependency of the gix umbrella in 0.83, so trimming means moving to subcrates, deferred). --uninstall is not added; with no config change left, uninstall is deleting one file, and CONTRIBUTING says so.

Full-repo audit is diagnostic and now reports 485 pre-existing hits on the merged tree (the docs/superpowers narrowing brought about 60 into view). --changed-vs origin/main is clean. Branch is merged up to origin/main.

[prk-Jr]

Summary

The installer redesign addresses most prior findings, but real Git/CLI reproductions show four remaining problems in commit-index selection, complete-host extraction, credential redaction, and hooksPath handling.

Blocking

• 🔧 P1 — Read the index Git supplies to the pre-commit hook — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:1195.
• 🔧 P1 — Finish extracting the authority before canonicalizing it — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:558.
• 🔧 P2 — Redact userinfo in the escaped URLs the scanner now recognizes — see inline at crates/trusted-server-cli/src/commands/dev/lint/domains.rs:2322.
• 🔧 P2 — Preserve an explicitly empty hooksPath during preflight — see inline at crates/trusted-server-cli/src/commands/dev/install_hooks.rs:174.

Validation and scope

The original-head CLI build, formatting, 195 focused unit tests, and 32 CLI E2E tests passed. Fresh fixture repositories reproduced all four findings. The illustrative index, redaction, and hooksPath fixes were tested individually, but the full cross-target release matrix was not rerun; they remain manual proposals, not one-click suggestions. Linux-only and Windows behavior were not tested locally.

CI Status

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