Skip to content

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

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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions