Skip to content

fix(provenance): reject a ref-type on a non-Azure-DevOps origin - #57

Open
gosharplite wants to merge 3 commits into
stacklok:mainfrom
gosharplite:fix/lint-ref-type-origin
Open

gosharplite wants to merge 3 commits into
stacklok:mainfrom
gosharplite:fix/lint-ref-type-origin

Conversation

@gosharplite

@gosharplite gosharplite commented Sep 27, 2026 •

Copy link
Copy Markdown

Fixes #54.

ADR-0019 and docs/10-vendoring.md both say modelith-ref-type applies only to Azure DevOps and is omitted for GitHub, but Header.validate (internal/provenance/provenance.go) only checked that the value was one of branch/tag/commit — never which origin it appeared on.

The fix

validate now reports a ref-type on any origin but dev.azure.com. The host set is closed, the same posture as an unknown fetch: method — a ref-type on a host whose API resolves an untyped ref on its own is a defect, not a value to ignore. Because deps refuses a header carrying any problem, this also stops deps update from silently dropping the key (recordedRefType returns "" for GitHub).

Placed in validate rather than the lint layer so every consumer agrees: modelith lint surfaces it as a provenance finding, and deps check/deps update refuse the header instead of acting on it.

Before / after (the issue's exact repro)

A GitHub-origin header with the key:

$ modelith lint gh-reftype.modelith.yaml
# before: 0 error(s), 0 warning(s)
# after:
  error   [semantic] (root): provenance header: line 6: provenance ref-type "tag" is recorded only for a dev.azure.com origin, and this file's origin is "https://github.com/acme/billing" — a host whose API resolves an untyped ref on its own carries no ref-type key; remove the line

A legitimate ADO header (origin dev.azure.com, ref-type: branch) and a GitHub header without the key both still lint clean (0 error(s), 0 warning(s)).

Review follow-ups (3 commits)

An independent architecture review ran three rounds on this branch. Round 1 and 2 raised findings, both addressed:

  • 98c498c — originHost now uses url.Hostname(), so an origin written with an explicit port (dev.azure.com:443) is not read as a different host; TestParse_Valid is table-driven over the ADO and GitHub header shapes.
  • d781627 — round 2 found that the above made internal/provenance and internal/deps disagree (provenance stripped the port, deps kept it), so a hand-written ported ADO origin passed lint but deps check refused it. Host normalization now has a single source of truth: provenance.NormalizeHost / provenance.OriginHost, consumed by ParseSource and sourceFromHeader; deps' duplicates are deleted.

Round 3 came back clean.

Tests

  • TestADR_0019_RefTypeIsOnlyValidOnAnADOOrigin (provenance) — a GitHub origin carrying the key is rejected (problem names the origin and points at the key's line); a GitHub header without the key, and an ADO origin with an explicit port, both parse clean.
  • TestVendored_RefTypeOnAGitHubOriginIsReported (lint) — the rule surfaces through modelith lint as a SeverityError, reproducing the issue.
  • TestOriginHost (provenance) and ported GitHub/ADO cases in the deps parse tables — pin the shared normalizer.
  • The vendored fixture carried the key on a GitHub origin — itself the defect — so its origin is now Azure DevOps.

Verification

go build ./..., go vet ./..., staticcheck@v0.7.0, and golangci-lint@v2.12.2 (0 issues) clean; go test -race ./... passes; lint-models and render-check green (no golden drift). The #54 repro and the ported-origin case were both checked against the built binary.

ADR-0019 and docs/10-vendoring.md say modelith-ref-type applies only to
Azure DevOps and is omitted for GitHub, but validate checked only the
value's membership in the closed set, never which origin it appeared on.
So a GitHub-origin header carrying the key linted clean, and deps update
then dropped it without saying so.

validate now reports a ref-type on any origin but dev.azure.com. The host
set is closed, matching the posture of an unknown fetch method. Because
deps refuses a header with any problem, update no longer drops the key
silently either.

The provenance fixture carried the key on a GitHub origin, which is
exactly the defect; its origin is now Azure DevOps, and a new test pins
that a GitHub origin with the key is rejected while one without it stays
valid.

Fixes stacklok#54

Signed-off-by: blevins darrin <darrinb765@gmail.com>
… the GitHub header shape in TestParse_Valid

The ref-type host check compared u.Host, which keeps an explicit port, so an
otherwise-valid ADO origin written as https://dev.azure.com:443/... was
rejected as a non-ADO host. Match on u.Hostname() instead, and cover the
ported origin in the ADR-0019 test.

TestParse_Valid becomes table-driven over the two header shapes that can
occur: an Azure DevOps header (which records the ref type) and a GitHub
header (which must not).

Addresses architect review round 1 on stacklok#54.

Signed-off-by: blevins darrin <darrinb765@gmail.com>
internal/provenance normalized an origin's host through url.URL.Hostname, which
drops an explicit port, while internal/deps (ParseSource's dispatch and
deps.originHost) normalized through url.URL.Host and kept it. The two therefore
disagreed about the same origin: a hand-written ADO origin written as
https://dev.azure.com:443/... passed `modelith lint` but `deps check`,
`deps update`, and `deps import` refused it as an unsupported host.

Export the provenance normalizers as NormalizeHost and OriginHost and make deps
consume them — ParseSource dispatches on provenance.NormalizeHost(u.Hostname())
and refresh's sourceFromHeader on provenance.OriginHost — deleting deps' own
copies, so host normalization has a single source of truth and the two commands
cannot disagree again. ParseSource's error text still reports u.Host, since it
echoes what the user typed rather than comparing it.

Covered by a table for OriginHost in provenance and by ported GitHub and ADO
cases in the deps parse tables; the ported-origin check in the ADR-0019 test is
tightened from a message scan to a strict "parses clean" assertion.

Addresses architect review round 2 on stacklok#54: it closes the ported-origin
divergence between lint and deps.

Signed-off-by: blevins darrin <darrinb765@gmail.com>
@gosharplite
gosharplite marked this pull request as ready for review September 27, 2026 10:03
@gosharplite
gosharplite requested a review from jbeda as a code owner September 27, 2026 10:03

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lint accepts modelith-ref-type on a GitHub-origin provenance header

2 participants