Add ts dev lint domains + ts dev install-hooks

[aram356]

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).
• Adds ts dev install-hooks — installs a managed pre-commit hook that runs ts dev lint domains --staged, with foreign core.hooksPath preflight, unmanaged-clobber refusal, and --force with timestamped backup.
• Refactors the existing ts dev leaf into a subcommand group: ts dev serve preserves the prior surface; lint and install-hooks are siblings.

Stacked on #669 — base branch is feature/ts-cli. Merge that PR first.

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. Exit codes: 0 clean, 1 violations, 2 environment error, 130 cancelled.

Test plan

• cargo fmt --all -- --check
• cargo clippy --workspace --exclude trusted-server-cli --all-targets --all-features -- -D warnings
• cargo clippy --package trusted-server-cli --target <host> --all-targets -- -D warnings
• cargo test --package trusted-server-cli --target <host> — 132 tests pass
• cd crates/js/lib && npx vitest run — 291 tests pass
• cd crates/js/lib && npm run format
• cd docs && npm run format
• cargo test --workspace --exclude trusted-server-cli — one pre-existing failure on the base (test_env_var_roundtrip_normalizes_integration_types) reproduces on origin/feature/ts-cli with zero files touched in trusted-server-core/ on this branch; not a regression from this work.

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

[ChristianPavilonis]

P1 – Correct the self-exclusion path

SELF_PATH omits the commands/ path component, so the component-aware Path::ends_with check cannot match this file's actual repository path. Consequently full, changed-vs-ref, staged, and explicit scans can report the linter's own intentionally disallowed-host fixtures; the installed staged hook can then reject commits that modify this source file. The regression test currently constructs the same incorrect src/dev/lint layout, so it does not cover the real path.

Suggested change

	constSELF_PATH:&str = "crates/trusted-server-cli/src/dev/lint/domains.rs";
	constSELF_PATH:&str = "crates/trusted-server-cli/src/commands/dev/lint/domains.rs";

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

[prk-Jr]

🔧 wrench — SELF_PATH names crates/trusted-server-cli/src/dev/lint/domains.rs, but this module ships at crates/trusted-server-cli/src/commands/dev/lint/domains.rs. Path::ends_with is component-aware, so the commands segment makes the suffix never match and the self-exclusion is dead code.

Measured against this head, with a binary built from it:

$ ts dev lint domains crates/trusted-server-cli/src/commands/dev/lint/domains.rs
crates/trusted-server-cli/src/commands/dev/lint/domains.rs:335: disallowed host test.com
crates/trusted-server-cli/src/commands/dev/lint/domains.rs:421: disallowed host c.evil
crates/trusted-server-cli/src/commands/dev/lint/domains.rs:641: disallowed host 2001:db8::1
... 38 disallowed host(s) found in 1 file(s).
exit 1

ts dev lint domains --changed-vs origin/main on this head reports 129 violations in 4 files, 38 of them from this file — so the Stage 2 CI mode would fail on the PR that introduces it.

The three unit assertions and the integration test assert the stale spelling, so they pass today and would fail if only the constant were fixed. That's why this can't be a one-click suggestion.

Proposed fix (apply manually — 5 sites across 2 files):

-const SELF_PATH: &str = "crates/trusted-server-cli/src/dev/lint/domains.rs";+const SELF_PATH: &str = "crates/trusted-server-cli/src/commands/dev/lint/domains.rs";

Then the same src/dev/lint → src/commands/dev/lint change at the doc comment on line 889, the not_scanned_paths entry on line 1752, and both paths in self_excludes_via_absolute_path_suffix (lines 1768 and 1774, including the notrusted-server-cli false-positive guard). In crates/trusted-server-cli/tests/lint_domains_cli.rs:482, rustfmt wants the longer path broken up:

let nested = temp
.path().join("crates/trusted-server-cli/src/commands/dev/lint");

I verified that exact patch in a scratch worktree: cargo fmt --all -- --check clean, cargo clippy -p trusted-server-cli --target aarch64-apple-darwin --all-targets -- -D warnings clean, cargo test -p trusted-server-cli 236 + 28 pass, self-scan drops to note: ... is excluded; skipping with exit 0, and --changed-vs origin/main drops from 129 to 91.

The stale spelling also appears in the two docs this PR adds (spec lines 527, 548, 841, 1155; plan lines 779, 780, 970, 983, 1042, 1053, …, 2488) — worth the same sweep so the next reader doesn't reintroduce the constant from the design doc.

[prk-Jr]

🔧 wrench — Setting core.hooksPath = .githooks also moves git away from .git/hooks, so every hook already living there stops running. The preflight above only inspects the core.hooksPath config value and .githooks/pre-commit; a populated .git/hooks is invisible to it, so this happens with no warning and without --force.

Reproduced against a binary built from this head:

$ git init -q . && printf '#!/bin/sh\nexit 1\n' > .git/hooks/pre-commit && chmod +x .git/hooks/pre-commit
$ ts dev install-hooks
Installed: pre-commit hook -> ./.githooks/pre-commit (runs /.../ts)
$ git add a.txt && git commit -m t
# exit 0 — the pre-existing hook that would have blocked this commit is now inert

For a repo whose .git/hooks/pre-commit is a secret scanner, installing a URL-host linter silently removes the stronger guard. That inverts the command's purpose, and the failure is invisible: nothing in the output hints the old hook stopped firing.

Proposed fix (apply manually — needs a new helper plus a call before the config write, so it can't be a single-range suggestion):

/// Executable, non-`.sample` hooks in the default `.git/hooks` directory/// that setting `core.hooksPath` would silence.fndisplaced_default_hooks(repo:&gix::Repository) -> Result<Vec<PathBuf>,Report<InstallHooksError>>{let dir = repo.git_dir().join("hooks");letmut found = Vec::new();let entries = match fs::read_dir(&dir){Ok(entries) => entries,Err(e)if e.kind() == std::io::ErrorKind::NotFound => returnOk(found),Err(e) => returnErr(Report::new(InstallHooksError::WriteHook).attach(e.to_string())),};for entry in entries.flatten(){let path = entry.path();if path.extension().is_some_and(|ext| ext == "sample"){continue;}if path.is_file() && is_executable(&path){
found.push(path);}}
found.sort();Ok(found)}

Then, alongside the existing core.hooksPath preflight: if displaced_default_hooks is non-empty and !force, return a new InstallHooksError::WouldSilenceDefaultHooks { paths } naming each file and pointing at --force; under --force, print the same list as a note: on stderr next to the existing displaced-core.hooksPath note. Same shape as the two guards already here, so the exit contract doesn't change.

[prk-Jr]

🔧 wrench — gix::open opens the repository at the given path; it does not discover upward the way git does. run passes env::current_dir(), so every mode fails outside the repo root:

$ cd crates/trusted-server-cli && ts dev lint domains --staged
environment error
├─▶ failed to open git repository
├─▶ ".../crates/trusted-server-cli" does not appear to be a git repository
╰─▶ Missing HEAD at '.git/HEAD'
exit 2

--changed-vs behaves the same (line 1190), as does full-repo mode (line 1563). install_hooks::run has the same problem via install_hooks(Path::new("."), ...):

$ cd sub && ts dev install-hooks
environment error
├─▶ failed to open git repository

Exit 2 is the "environment is broken" code, so a developer who runs the linter from the crate directory they're editing gets a message implying the checkout isn't a git repo. The pre-commit hook path is unaffected (git invokes hooks with cwd at the worktree root), which is why the test suite — every case builds a fixture repo and runs at its root — doesn't catch it.

Proposed fix (apply manually — 4 call sites across 2 files):

let repo = gix::discover(repo_path).change_context(DomainsLintError::OpenRepo)?;

at lines 995, 1190, and 1563, and in install_hooks::install_hooks (line 185). While there, install_hooks should derive hooks_dir from the discovered repo.workdir() rather than the relative . it is handed — that also cleans up the ./.githooks/pre-commit spelling in the success line. Consider one #[test] that runs a collector from a nested subdirectory of the fixture repo, since no current test would fail on a regression here.

[prk-Jr]

⛏ nitpick — The remediation hint points at the pre-move path, so a contributor who hits a violation is told to edit a file that doesn't exist. Same root cause as the SELF_PATH finding, but harmless to apply on its own — I verified this single-line change in isolation (cargo fmt --all -- --check, cargo clippy -p trusted-server-cli --all-targets -- -D warnings, cargo test -p trusted-server-cli 236 + 28 pass, no drift):

Suggested change

	crates/trusted-server-cli/src/dev/lint/domains.rs.",
	crates/trusted-server-cli/src/commands/dev/lint/domains.rs.",

[prk-Jr]

♻️ refactor — temp-env is added here but never referenced: no temp_env usage anywhere under crates/trusted-server-cli/ (the Cargo.toml line is the only hit in the crate). assert_cmd, predicates, and tempfile are all genuinely used; this one looks like a leftover from an earlier draft of the fixture strategy.

Proposed fix (apply manually — dropping the line also rewrites Cargo.lock, so it can't be a one-click suggestion):

 [target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
assert_cmd = "2"
predicates = "3"
-temp-env = { workspace = true }
tempfile = { workspace = true }

then cargo check -p trusted-server-cli --target <host> to refresh Cargo.lock (the temp-env entry disappears from the crate's dependency list). I verified the removal in a scratch worktree: fmt clean, clippy clean, full CLI suite passes.

[prk-Jr]

🤔 thinking — These four (gix, gix-config here, assert_cmd and predicates in dev-dependencies) are the only direct deps in the workspace declared with inline versions; the root [workspace.dependencies] centralizes 95 others, and every other crate uses { workspace = true }. The comment explains why the versions are pinned, which is the valuable part — but with the version living in a leaf manifest, a second crate that later wants gix (an integration-tests harness, say) can silently resolve a different minor and reintroduce the duplicate-version problem this comment is guarding against.

Moving the four version specs to [workspace.dependencies] and keeping the feature list plus the rationale comment here (gix = { workspace = true, features = [...] }) preserves the intent and makes the pin enforceable workspace-wide. Non-blocking — flagging it because the comment implies the pin matters more than a leaf manifest can guarantee.

[prk-Jr]

🤔 thinking — This pattern accepts a host with no dot at all ([A-Za-z0-9][A-Za-z0-9.\-]*), while the protocol-relative pattern below requires \.[A-Za-z]{2,}. Single-label hosts therefore become violations only in the absolute form:

$ cat probe.ts
const a = "http://myservice/health";
const b = "https://origin/api";
const c = "https://registry:5000/img";
$ ts dev lint domains probe.ts
probe.ts:1: disallowed host myservice
probe.ts:2: disallowed host origin
probe.ts:3: disallowed host registry
exit 1

Docker-compose service names, Kubernetes in-cluster names, and container-registry hostnames all look like that, and this repo's integration fixtures are exactly where such URLs live (the scanned extensions include Dockerfile*, yml, yaml, and .env*). Under Stage 1 that's diagnostic noise; under Stage 2 it becomes a gate that fails on legitimate compose files, and the only escape is a per-line marker.

Two coherent options: require a dotted suffix in this pattern too (matching the protocol-relative rule, at the cost of missing https://evilhost/ single-label exfiltration), or keep flagging them and add the handful of infrastructure names to EXACT_HOSTS as they appear. Either way it's worth stating the intent in the spec's host-shape section, which currently documents the dotted-suffix rule only for the protocol-relative regex.

[prk-Jr]

♻️ refactor — This file and crates/trusted-server-cli/src/commands/dev/lint/test_support.rs have byte-identical bodies (~200 lines: test_signature, init_repo, stage_all, collect_files, rel_path_to_bstring, commit_all, commit_all_as_branch, commit_index_to_ref, create_and_checkout_branch); only the header comments differ. diff between the two bodies reports nothing else. Sync-by-comment across 200 lines of gix plumbing is the kind of duplication that drifts silently — a fix to stage_all in one copy leaves the other subtly different, and both are load-bearing for correctness tests.

Rust has a first-class escape for exactly this: include the crate-internal module by path from the integration test, so there is one definition.

// crates/trusted-server-cli/tests/common/mod.rs#[path = "../src/commands/dev/lint/test_support.rs"]mod test_support;pubuse test_support::*;

(The pub(crate) items become pub within the test binary's own crate root, so the visibility note in the current header no longer applies.) Also note both headers reference the pre-move path src/dev/lint/test_support.rs, which no longer exists — same stale-path sweep as the SELF_PATH finding.

[prk-Jr]

🤔 thinking — added_lines runs every blob through String::from_utf8_lossy, so --staged and --changed-vs scan binary content, while full_repo_lines explicitly skips it (ErrorKind::InvalidData → note: skipping ...: binary content). A committed file with a scanned extension but non-UTF-8 bytes — a .json fixture with a stray byte, a minified .js with a Latin-1 escape — is silently lossy-decoded here, and any URL-looking byte run in it becomes a violation with a line number that doesn't correspond to anything a human can read.

The asymmetry looks unintentional rather than a documented trade-off (the spec's edge-case list treats binary content as a skip case). Cheapest alignment is a from_utf8 check on new before diffing: on Err, emit the same warn_skip-style note and return no lines, so all four modes agree on what "binary" means.

[prk-Jr]

🌱 seedling — write_index_to_tree calls editor.write(), which persists a real tree object into .git/objects — so ts dev lint domains --staged mutates the object database on every invocation, including every pre-commit hook run. It's what git write-tree does and the objects are unreferenced and gc-able, so nothing breaks; but a linter that only reads is a nicer neighbour, and on a busy repo this accumulates loose objects until the next gc.

If it ever matters, gix's in-memory object store (repo.objects.enable_object_memory() / writing into a memory-backed ODB, per gix's "in-memory objects" pattern) lets the tree exist only for the diff. Worth a note in the module docs either way, since "the linter wrote objects into my repo" is surprising if you go looking.