fix(provenance): reject a ref-type on a non-Azure-DevOps origin - #57
Open
gosharplite wants to merge 3 commits into
Open
gosharplite wants to merge 3 commits into
gosharplite wants to merge 3 commits into
Conversation
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
marked this pull request as ready for review
September 27, 2026 10:03
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #54.
ADR-0019 and
docs/10-vendoring.mdboth saymodelith-ref-typeapplies only to Azure DevOps and is omitted for GitHub, butHeader.validate(internal/provenance/provenance.go) only checked that the value was one ofbranch/tag/commit— never which origin it appeared on.The fix
validatenow reports aref-typeon any origin butdev.azure.com. The host set is closed, the same posture as an unknownfetch:method — aref-typeon a host whose API resolves an untyped ref on its own is a defect, not a value to ignore. Becausedepsrefuses a header carrying any problem, this also stopsdeps updatefrom silently dropping the key (recordedRefTypereturns""for GitHub).Placed in
validaterather than the lint layer so every consumer agrees:modelith lintsurfaces it as a provenance finding, anddeps check/deps updaterefuse the header instead of acting on it.Before / after (the issue's exact repro)
A GitHub-origin header with the key:
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—originHostnow usesurl.Hostname(), so an origin written with an explicit port (dev.azure.com:443) is not read as a different host;TestParse_Validis table-driven over the ADO and GitHub header shapes.d781627— round 2 found that the above madeinternal/provenanceandinternal/depsdisagree (provenance stripped the port, deps kept it), so a hand-written ported ADO origin passedlintbutdeps checkrefused it. Host normalization now has a single source of truth:provenance.NormalizeHost/provenance.OriginHost, consumed byParseSourceandsourceFromHeader; 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 throughmodelith lintas aSeverityError, reproducing the issue.TestOriginHost(provenance) and ported GitHub/ADO cases in the deps parse tables — pin the shared normalizer.vendoredfixture 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, andgolangci-lint@v2.12.2(0 issues) clean;go test -race ./...passes;lint-modelsandrender-checkgreen (no golden drift). The #54 repro and the ported-origin case were both checked against the built binary.