Repository navigation
feat(forge): widen the config diagnostic to hosts that lost exact-label classification - #3672
worktrunk-bot wants to merge 3 commits into
Conversation
…el classification Since 0.71.0 (#3662) forge hosts classify by exact DNS label, which drops self-hosted instances whose hostname carries the forge name only inside a label (gitlab-internal.company.com, github-enterprise.acme.com, mygithub.life-white.uk). Only the branded single-label SSH alias announced the forge.platform remedy; a hyphenated hostname simply stopped reporting CI with no message. Widen LegacyForgeAlias detection to any host the pre-0.71.0 substring rule would have matched but the exact-label boundary no longer does. The diagnostic now words itself by shape (SSH alias vs hostname) and covers Azure DevOps service domains carried as substrings too. Additive: it changes no classification and never feeds provider dispatch. Closes #3671
worktrunk-bot
left a comment
worktrunk-bot
left a comment
There was a problem hiding this comment.
The widening is clean and correct: dispatch stays on ForgeKind::from_host's exact-label boundary (guarded first, before any diagnostic path), the new legacy_substring_platform mirrors that precedence (Azure fixed-domains → github → gitlab → gitea), and both message shapes plus legacy_substring_platform are covered by the new tests. The #3662 lookalikes still return None from dispatch while surfacing a config hint — matches the issue's intent. Verified cargo test --lib ci_platform passes locally.
One minor doc-accuracy nit, outside the diff so noting it here rather than inline: this PR widens what legacy_forge_alias() (the impl Repository accessor in src/git/ci_platform.rs) returns, but its summary line still reads Return a diagnostic for a legacy branded SSH alias on the primary remote. — it now also returns hostname-shaped diagnostics (github-enterprise.acme.com, mygithub.life-white.uk). The LegacyForgeAlias type doc was updated to match; the accessor's one-liner wasn't. Happy to push a one-word reword (branded SSH alias → legacy forge host) if you'd like, though it's cosmetic and not worth a CI cycle on its own.
|
Done — pushed the reword in a5fb8bb. The accessor's summary now reads |
… label (#3673) Reverts the branded half of the exact-label classifier and deletes the diagnostic built to explain it. `github-enterprise.acme.com`, `mygithub.life-white.uk`, `gitlab-internal.company.com`, and the `github-personal` SSH alias resolve to their forge again, so CI status, `wt switch --prs`, and `repo.provider` work with no config. ## Why the label boundary goes It looked like an ownership check and wasn't one. An attacker controls their own DNS, so `github.attacker.example` has the exact label `github` and classified fine; `gitlab.evil.co.uk` likewise. What the rule actually excluded was the self-hoster who put the brand in a hyphenated name. It failed open for the adversary and closed for the customer. The residual case for it doesn't survive either. The hostname comes out of the user's own `.git/config`, and whoever can put a host there can put code there too — the trust decision happens at clone time, and by the time worktrunk reads the remote the user is already building from it. All the classification decides is which forge CLI (`gh`, `glab`, `tea`, `az`) runs against it. So the rule is recall-first: any host carrying `github`, `gitlab`, or `gitea` matches, first match winning. The cost is a host that merely sounds like a forge getting a forge CLI run at it, which surfaces as that CLI's error rather than as silence — the better of the two failures, and `forge.platform` overrides it. ## What stays Azure DevOps keeps suffix matching on its two service domains, and for a reason unrelated to security: those are service domains rather than a brand in the host, so every real hosted instance already matches, and the on-prem edition carries neither string. `dev.azure.com.attacker.example` and `evil-visualstudio.com` are outside the domains and carry no brand to fall back on, so they stay unclassified. Userinfo still resolves to the network host, so `https://github.com@attacker.example/…` is `attacker.example`. ## What goes `LegacyForgeAlias`, `Repository::legacy_forge_alias`, `legacy_forge_alias_diagnostic`, and its three emit sites in `wt list`, `wt switch --prs`, and `wt config show --full`. Every host the diagnostic fired on now classifies, so it could only ever return `None`. A host with no brand at all still reaches the existing generic hint, which is the right message there — there is no platform to infer. The end-to-end warning-dedup test goes with it, since no warning is raised from both the collect and `--prs` threads any more; `stash_warning_preserves_order` keeps the mechanism covered. ## Docs `## Forge platform` in `src/cli/mod.rs` described the override as being for SSH aliases and self-hosted instances, which now detect on their own. It states the rule and scopes the override to hosts carrying no brand — a Forgejo instance at `forge.example.com`. Mirrors, `dev/wt.example.toml`, and the two `config` help snapshots regenerate from it. --- The branch's history has a false start — the first commit widened the diagnostic, the second deletes it in favour of relaxing classification — plus a merge of `main` after v0.71.0 shipped. The net diff is the second approach; it all squashes on merge. Also supersedes [#3672](#3672), the triage bot's PR for the same issue — it widens the diagnostic rather than removing the need for one, so it should be closed too. Closes #3671 > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Superseded by #3673, now merged. That PR took the other route on the same issue: rather than widening the diagnostic, it relaxed classification itself back to a substring test, so Azure DevOps kept suffix matching, matching the aside in this PR's reasoning but for a different reason: those are service domains rather than a brand in the hostname, so recall there was already complete. Thanks — the diagnostic wording and the Azure analysis here were both useful input.
|
Problem
Since 0.71.0 (#3662), forge hosts are classified by exact DNS label. That correctly rejects lookalikes, but it also drops legitimately-named self-hosted instances whose hostname carries the forge name only inside a label —
gitlab-internal.company.com,github-enterprise.acme.com,mygithub.life-white.uk. Those lose CI status inwt list --full,wt switch --prs, andrepo.providerinwt list --format=json.The remedy is one line —
forge.platformin.config/wt.toml— but only one shape announced itself:LegacyForgeAlias::from_urlrequired an SSH URL and a dotless host and a forge prefix, so it caught the branded multi-account alias (git@github-personal:owner/repo.git) and nothing else. A dotted hyphenated host got no message — CI simply stopped appearing, reading as a bug rather than a configuration gap.Solution
Widen
LegacyForgeAliasdetection to any host the pre-0.71.0 substring rule would have matched but the exact-label boundary no longer classifies: if the host has no exact forge label and a case-insensitive substring isgithub/gitlab/gitea(or an Azure DevOps service domain carried as a substring), the existingforge.platformhint fires with the inferred platform.The diagnostic now carries a shape (
SshAliasvsHostname) so it words itself for the case at hand — "SSH host alias …" for the branded alias (unchanged), "Host …" for a hyphenated/forge-named hostname. This is additive: it changes no classification and never feeds provider dispatch, so it keeps the exact-label security boundary #3662 bought. Per the issue's aside, the same widening covers Azure DevOps.Testing
test_legacy_forge_alias_widens_to_hostnames_that_lost_exact_label(new) coversgitlab-internal.company.com,github-enterprise.acme.com,mygithub.life-white.uk, hyphenated SSH hosts, an Azure substring host, and assertsForgeKind::from_hoststill rejects each (dispatch unchanged).test_legacy_forge_alias_is_diagnostic_only_for_branded_ssh_aliases(updated) now asserts theSshAliasshape and keeps the genuinely-unrecognized hosts (bitbucket.org,codeberg.org, exact-labelgithub.life-white.uk) returningNone.fetch_open_prs_explains_legacy_forge_hostname_configuration(new) confirms the--prsbail ongitlab-internal.company.comuses the "Host …" wording and theforge.platform = "gitlab"remedy.The existing SSH-alias snapshot tests (
config_show,list,switch_picker_dry_run) usegithub-personaland keep their byte-identical wording, so no snapshot regeneration was needed.Notes
Detection lives in
src/git/ci_platform.rs; the shape-aware message islegacy_forge_alias_diagnostic. CHANGELOG left untouched — entries are curated at release time per thereleaseskill.Closes #3671 — automated triage