Skip to content

Add a CI gate for non-allowlisted URL hosts on changed lines #1215

Closed

Description

@aram356

Description

The pre-commit hook that #733 ships (ts dev install-hooks → ts dev lint domains --staged) is the only enforcement mechanism today. The spec says so outright: "No CI gate in v1. The pre-commit hook is the only enforcement mechanism."

That leaves two holes:

  • The hook is bypassable with git commit --no-verify.
  • The hook only runs for developers who have actually run ts dev install-hooks. A fresh clone, a new contributor, or a commit made through the GitHub web UI has no protection at all.

So nothing prevents a disallowed host from reaching main. Trusted Server runs at the edge on behalf of publishers, which makes an unreviewed outbound host a supply-chain concern rather than a style issue — the same framing as #1160.

No CI job checks domains today: no workflow file references the linter, and #733 touches no files under .github/.

Proposed solution

Stage 2 of the design spec already pins the shape: a GitHub Actions job that runs

ts dev lint domains --changed-vs $GITHUB_BASE_REF

on every pull request. Same delta-only enforcement as the hook, but unbypassable.

Requirements the spec calls out:

  • actions/checkout@v4 with fetch-depth: 0, or an explicit fetch of $GITHUB_BASE_REF.
  • Reuse the host-target CI lane, since the ts binary is host-target only (the workspace default target is wasm32-wasip1, where the CLI crate is an empty shell).

Implementation note: the lane to reuse is the existing test-cli job in .github/workflows/test.yml (cargo test (ts CLI, native)), which already builds the CLI for the host triple via ./scripts/test-cli.sh. Its actions/checkout@v4 step does not set fetch-depth: 0 today, so that has to be added or the base ref fetched explicitly — otherwise --changed-vs has no base commit to resolve and the job cannot compute a delta.

Why a gate and not just the hook: during review of #733, reviewers found four separate cases where the linter silently exited 0 on a disallowed host — userinfo spans crossing string boundaries, IPv4-mapped IPv6 literals, URL-valid punctuation such as ! and ; truncating the host to an allowlisted prefix, and consecutive single-quoted URLs causing a skipped host. Those are exactly the class of bug a gate running against real PR diffs surfaces, instead of depending on a reviewer hand-crafting probes.

Follow-on: Stage 3

Stage 3 (the full-repo audit as a gate, rather than delta-only) stays deferred and is blocked on the pre-existing-violation cleanup in #1216. Per the spec, the choice between (a) cleaning every violation and gating the full audit and (b) snapshotting a baseline file and subtracting it is deferred until Stages 1 and 2 are stable. Tracking it here as a follow-on rather than as its own ticket.

Done when

  • A CI job runs ts dev lint domains --changed-vs $GITHUB_BASE_REF against the PR base ref on every pull request.
  • The job fails the PR when a changed line introduces a non-allowlisted host.
  • The base ref is fetched (fetch-depth: 0 or an explicit fetch) so --changed-vs resolves.
  • The job reuses the existing host-target lane rather than adding a second Rust toolchain install.
  • A deliberate test PR that adds a disallowed host is shown to fail the gate.
  • A decision is recorded on Stage 3: clean-and-gate vs baseline file.

Affected area

.github/workflows, crates/trusted-server-cli, CI / Tooling.

Design

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions