Skip to content

Commit 1debc22

Browse files
authored
audit(1to1): adjudicate the routes no action reaches and gate on them (#1642)
Refs #1595 Refs #1604 Refs #1605 Refs #1606 Refs #1607 Refs #1608 Refs #1609 Refs #1610 Refs #1611 Refs #1612 Refs #1613 Refs #1614 Refs #1615 Refs #1616 Refs #1617 Refs #1618 Refs #1209 Refs #1619 Refs #1620 Refs #1621 Refs #1622 Refs #1623 Refs #1624 Refs #1625 Refs #1626 Refs #1627 Refs #1628 Refs #1629 Refs #1630 Refs #1631 Refs #1632 Refs #1633 Refs #1634 This is the seventh layer of the auditor stack. The sixth added R-ROUTE and gated on stale declarations only, which left 730 routes of the 19.5.0 record that no action sends and nothing answered. This layer answers every one of them and turns on the full gate. ## What the check detects `-scope=routes` now fails on a finding of any class (a route nothing answers, an unknown skip reason, an internal route left in, a contradicted decision), as it already did on a declaration that no longer describes the tree. The gate reads those classes from `findingClasses`, so a class added there is gated without a second list to keep in step. A re-pin that brings a route, or an action that stops sending one, fails the gate until the change that causes it decides the route, and every family row pins how many routes it answers. One declaration problem is new: a `tracked` row over a route whose client-go method `acceptedMissingMethods` decides `COVERED_GENERIC` or `INTENTIONAL_SKIP_*`. The row says an action is still to implement and the decision that the route needs none, so one of the two no longer describes the tree. ## How the routes are answered I applied the decisions of 2026-10-27. `covered-by-graphql` is a REST route whose operation an action does over GraphQL, the action checked in the catalog and in the request record: 49 routes. They are the `/-/` epic routes and the child epic links, the five work item operations and a child's attachment and detachment behind `work_item_rest_api`, a group's work item's discussions, notes and children (13 routes the `group.epic_*` actions cover, since their GraphQL reads the group's work item by its iid whatever its type), a vulnerability's confirmation, dismissal, resolution, revert and read, and a pipeline's findings. A project's and a namespace's work item discussions, notes and children stay tracked, and so does the resolution of a discussion, which no action does. `client-protocol` is the protocol a package manager's or MLflow's own client speaks, 150 routes, except a download, which is tracked. The line is what the route serves, not how GitLab words its summary: what a package or a model version holds (one of its files, its archive, its source or a file inside it, such as a Go module's `go.mod`) is a download, while an index or a metadata document a client reads to find and resolve packages, and the URL or the location of the content, are the protocol's. So Debian's `Release` and `Packages` files, Helm's `index.yaml`, RPM's `repodata` and RubyGems' spec indexes and gemspecs are `client-protocol`. The witness holds every `client-protocol` route whose summary says download to a list that names what each serves instead, and every entry of that list and every protocol root to a route it answers. `tracked` is everything else, 534 routes, under 32 issues: one for each of thirty catalog domains and one for each of two domains the catalog does not hold yet, the virtual registries and GitLab Duo. The fifteen `public_endpoint` routes and every administrator, GitLab.com and tier route are among them, and a route's reason says it is public exactly when its skip reason is `public_endpoint` (`POST /glql` is not; `GET /glql/schema` is). Seventeen of the tracked routes were settled by `acceptedMissingMethods`: six `COVERED_GENERIC` (a group's and a project's direct members, a group's subgroups, the delete of an LDAP link by its `cn`, a merge request's changes and the `PUT` of the error tracking settings) and eleven `INTENTIONAL_SKIP_*` (nine downloads, the streamed repository archive and the deprecated write of a merge request's approvals). A route still to implement is no method R-ACTION may settle, so those entries leave the table and R-ACTION lists the seventeen methods as its backlog, 18 missing methods with `TerraformStates.List`, which was there already; `Orbit.GetDsl` was the nineteenth until the Orbit migration, and is decided `SUPERSEDED_UPSTREAM` now. The creation of a vulnerability keeps its `COVERED_GRAPHQL` service decision, which does not carry because no GraphQL action creates one. The two runner token routes and the integrations list carry no decision at all: they are tracked rows whose reason names `runner_token_auth` or the `/services/` twin. The table moved to `family_table.go`, written with five constructors (`tracked`, `download`, `publicEndpoint`, `protocol`, `overGraphQL`). It has 331 rows, more than the design estimated, because no two rows may answer one route: downloads sit inside the protocol trees, so each protocol is split around its downloads, and a group's work item routes are split from a project's and a namespace's. The Orbit rows are as the Orbit migration (#1585) left Orbit, all under #1209, the issue that already tracked the Orbit routes no action reaches. The five routes the `orbit.*` actions sent until GitLab.com removed them (`GET /orbit/status`, `/orbit/schema`, `/orbit/tools`, `/orbit/schema/dsl` and `/orbit/graph_status`) inherit `SUPERSEDED_UPSTREAM` from the layer below, and `POST /orbit/mcp`, which the actions send in their place, is reached, so the `/orbit/mcp` row tracks the endpoint's SSE listener alone, `GET /orbit/mcp`. `GET /orbit/agent/commands`, which `orbit.dsl` read until GitLab.com removed it too and which no client-go method sends, shares a row with the invocation of a command; the query templates, the named templates, the response format guidance and the skills stay tracked, the reasons of the two GitLab.com removed in favour of GraphQL saying so. ## Witness tests, red then green On the parent as it stood before the stack was rebased onto #1585 (9a1b315fb), with this layer's tests, a stand-in for the four identifiers they need (two issue constants, `publicTail` and `Summary.counters`) and without its table or gate: ```text $ go test -count=1 -run "TestSummary_Clean|TestJudgeAll_AFamilyRow|TestRun_Repository" ./cmd/audit_1to1/internal/routes/ --- FAIL: TestSummary_Clean_PassesOnlyWithoutAFindingOrAStaleDeclaration (0.00s) --- FAIL: TestSummary_Clean_PassesOnlyWithoutAFindingOrAStaleDeclaration/an_unanswered_route (0.00s) analyze_test.go:301: clean({... Unanswered:1 ...}) = true, want false (and the unknown skip reason, the two internal classes and the contradicted decision) --- FAIL: TestSummary_Clean_FailsOnEveryFindingClassAndOnNoOther (0.00s) --- FAIL: TestSummary_Clean_FailsOnEveryFindingClassAndOnNoOther/unanswered (0.00s) analyze_test.go:318: one route of class unanswered: clean() = true, want false (and the other four finding classes) --- FAIL: TestJudgeAll_AFamilyRow_AnswersItsRoutesUntilTheyChange (0.02s) --- FAIL: TestJudgeAll_AFamilyRow_AnswersItsRoutesUntilTheyChange/a_tracked_row_over_a_route_a_COVERED_GENERIC_decision_settles (0.00s) analyze_test.go:575: stale = [], want ["family GET /gizmos tracks GET /gizmos, and Gizmos.ListGizmos is decided COVERED_GENERIC, which says the route needs no action of its own: an action still to implement is no method acceptedMissingMethods settles"] (and the INTENTIONAL_SKIP_BINARY case, and the cases whose gate the parent passes with findings left) --- FAIL: TestRun_Repository_DeclaresTheRoutesADecisionDoesNotCarryOnto (0.22s) analyze_test.go:960: GET /groups/:id/members = {Route: Class: ...}, want it tracked by a family row whose reason names "", with the decisions "Groups.ListGroupMembers unadjudicated" (and the other 21 routes of the witness) --- FAIL: TestRun_Repository_HoldsTheDuoWorkflowRoutesToTheTokenThatCallsThem (0.16s) analyze_test.go:1008: composite identity = [...], 0 a user's token calls tracked by the Duo row and [... unanswered ...] otherwise, want [...], 14 and none --- FAIL: TestRun_Repository_AdjudicatesEveryRouteNoActionReaches (0.16s) analyze_test.go:1089: declared map[tracked:3] under 2 issues; want map[client-protocol:150 covered-by-graphql:49 tracked:535] under one issue for each of 30 catalog domains and of the virtual registries and GitLab Duo, which it does not hold yet analyze_test.go:1092: 0 public_endpoint routes tracked, want 15; ... analyze_test.go:1095: ...; roots and listed downloads no client-protocol route answers: ["/group/:id/-/packages/" "/groups/:id/-/packages/" "/packages/" ...] --- FAIL: TestRun_Repository_HoldsTheWorkItemRoutesToTheActionsThatDoTheirOperation (0.14s) analyze_test.go:1174: work item routes by scope and category = map[], want map[groups:map[covered-by-graphql:20 tracked:10] namespaces:map[covered-by-graphql:7 tracked:23] projects:map[covered-by-graphql:7 tracked:23]]; ... FAIL ``` Since the rebase the witness of the decisions that do not carry holds 21 routes, not 22, and the count it wants is 534 tracked: `GET /orbit/schema/dsl` is inherited now, with the other four Orbit routes GitLab.com removed. With this layer's gate check and its seventeen `acceptedMissingMethods` entries still in place, `TestRun_Repository_GateIsGreen` names the contradiction on the real tree, six `is decided COVERED_GENERIC`, nine `is decided INTENTIONAL_SKIP_BINARY` and two `is decided INTENTIONAL_SKIP_OTHER`: ```text analyze_test.go:837: the gate fails: 0 routes no action reaches are left findings, among them [], ...; and the stale declarations ["family * /groups/*/uploads/** tracks GET /groups/:id/uploads/:secret/:filename, and GroupMarkdownUploads.DownloadGroupMarkdownUploadBySecretAndFilename is decided INTENTIONAL_SKIP_BINARY, which says the route needs no action of its own: ..." ...] ``` Here: ```text --- PASS: TestSummary_Clean_PassesOnlyWithoutAFindingOrAStaleDeclaration (0.00s) --- PASS: TestSummary_Clean_FailsOnEveryFindingClassAndOnNoOther (0.00s) --- PASS: TestJudgeAll_AFamilyRow_AnswersItsRoutesUntilTheyChange (0.02s) --- PASS: TestRun_Repository_GateIsGreen (0.88s) --- PASS: TestRun_Repository_HoldsTheRoutesClientGoLaterModels (0.20s) --- PASS: TestRun_Repository_DeclaresTheRoutesADecisionDoesNotCarryOnto (0.22s) --- PASS: TestRun_Repository_HoldsTheDuoWorkflowRoutesToTheTokenThatCallsThem (0.27s) --- PASS: TestRun_Repository_AdjudicatesEveryRouteNoActionReaches (0.26s) --- PASS: TestRun_Repository_HoldsTheWorkItemRoutesToTheActionsThatDoTheirOperation (0.25s) --- PASS: TestDeclares_ATrackedRowOverARouteADecisionSettles_IsAProblem (0.00s) ``` The witness is sensitive to the version on purpose: the client-go bump and every re-pin move its counts, and that change is the evidence the triage was run again. ## On main 2137 routes: 1007 reached, 20 outside the API, 186 skipped, 94 by a slug, 49 aliases, 48 inherited and 733 declared by 331 rows, with no finding and no stale declaration. Each tracked route is in the text of its domain's issue with its row, its summary and the client-go methods that send it, if any. ## Checks `go test ./... -count=1` with the snapshot parity deferred, and the `audit_1to1` suites on the real tree without it. 100% statements and every condition both ways on `routes` (gobco 242 of 242) and on `actions` (22 of 22); gremlins Lived 0 and Not covered 0 on `routes` (181 killed) and on `actions` (16 killed). `golangci-lint` is clean on `cmd/audit_1to1/...`, and the `make analyze` steps the layer touches pass: the routes, params and paths gates, the sdk gate, the subtest, goroutine, unread-constant, closed-server, godoc and test-name gates, the documented tool names and `golangci-lint fmt`, as do markdownlint, the table formatter, the doc link check and the em dash check. `gen_testing_docs --check` is regenerated at the top of the stack, as below this layer. The two pieces of work this layer kept clear of have merged since, the keyset half of the live record (#1574) and merge request diff pagination (#1584), and so has the Orbit migration (#1585), which stopped sending five Orbit routes this layer counted reached (`GET /orbit/agent/commands`, `/orbit/graph_status`, `/orbit/schema`, `/orbit/status` and `/orbit/tools`) and started sending `POST /orbit/mcp`, which the `* /orbit/mcp` row tracked. The stack is rebased onto main after all three, the Orbit rows are redone as above, and every figure in this description is measured there except the red run, which is quoted from the parent before that rebase. --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>
1 parent 44fd2b9 commit 1debc22

15 files changed

Lines changed: 1380 additions & 159 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,10 @@ jobs:
253253
if: env.FRESHNESS == 'checked'
254254
run: make audit-1to1-params
255255
# Every route the live record mounts, held to an action that sends it
256-
# or a reason none does. The findings are a report until the triage
257-
# adjudicates them; it fails on a declaration that no longer describes
258-
# the tree. It reads the request record the derivation commits, so it
259-
# is deferred with R-GRANT.
260-
- name: Check the declarations of the routes no action reaches (R-ROUTE)
256+
# or a reason none does. It fails on a route nothing answers and on a
257+
# declaration that no longer describes the tree. It reads the request
258+
# record the derivation commits, so it is deferred with R-GRANT.
259+
- name: Check that every live route is reached or answered (R-ROUTE)
261260
if: env.FRESHNESS == 'checked'
262261
run: make audit-1to1-routes
263262
# The Orbit twin of the record above: what GitLab.com's Orbit routes

‎Makefile‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2011,11 +2011,12 @@ audit-1to1-params-report:
20112011
## it or a reason none does (R-ROUTE): outside the API, a skip reason the closed table
20122012
## excludes, a slug the caller names, a deprecated /services/ alias, a decision about its
20132013
## client-go methods that holds without a request, or a family row of
2014-
## cmd/audit_1to1/internal/routes/families.go. Anything else is a finding, reported with
2015-
## the client-go methods that send it. It FAILS on a declaration that no longer describes
2016-
## the tree: a family row that answers no route, another number than it pins, a route
2017-
## another row answers, or that lacks what its category needs; a skip row no route
2018-
## carries, or a reason a route carries that no row holds; a skip exception or a GraphQL
2014+
## cmd/audit_1to1/internal/routes/family_table.go. Anything else is a finding, reported
2015+
## with the client-go methods that send it, and it FAILS on a finding as it does on a
2016+
## declaration that no longer describes the tree: a family row that answers no route,
2017+
## another number than it pins, a route another row answers, or that lacks what its
2018+
## category needs; a skip row no route carries, or a reason a route carries that no row
2019+
## holds; a skip exception or a GraphQL
20192020
## actions row that answers no route; a slug rule that answers nothing. It reads the
20202021
## committed request record, the live record and the pinned client-go; CI runs it under
20212022
## the FRESHNESS deferral with R-GRANT.

‎cmd/audit_1to1/internal/actions/analyze.go‎

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
const (
1919
coveredGraphQLEpicDsc = "COVERED_GRAPHQL. Epicdiscussions pkg"
2020
coveredGraphQLEpicNts = "COVERED_GRAPHQL. Epicnotes pkg"
21-
skipBinaryBytes = "INTENTIONAL_SKIP_BINARY. File bytes"
2221
coveredGraphQLEpics = "COVERED_GRAPHQL. Epics pkg"
2322
coveredGenericDeprPrj = "COVERED_GENERIC. Deprecated; project variant"
2423
)
@@ -111,12 +110,16 @@ func buildReport(root string, gapsOnly bool) (report, error) {
111110
// method of a client-go service interface, called in place or handed on as a
112111
// method value, so it misses: RAW REST handlers (client.GL().NewRequest+Do with
113112
// a path string), GraphQL handlers (ADR-0006), and capabilities covered by a
114-
// superseding/generic variant. It also can't know which endpoints are
115-
// INTENTIONALLY not exposed (binary file transfer, CI-job-token self-lookup).
116-
// Each entry carries a category+rationale. Key = "<Service>.<Method>" (service
117-
// name without the ServiceInterface suffix). Methods NOT listed here and still
118-
// uncovered are the genuine new-tool backlog, and an entry whose method a
119-
// handler reaches is reported as stale.
113+
// superseding/generic variant. It also can't know which endpoints a personal
114+
// access token cannot use (a CI job token's self-lookup). Each entry carries
115+
// a category+rationale. Key = "<Service>.<Method>" (service name without the
116+
// ServiceInterface suffix). Methods NOT listed here and still uncovered are
117+
// the genuine new-tool backlog, and an entry whose method a handler reaches
118+
// is reported as stale. A route no action sends is not settled here: a
119+
// download, a route another one covers only generically and a deprecated
120+
// write GitLab still serves are actions to implement, which R-ROUTE tracks,
121+
// and its gate refuses a tracked route whose method this table decides
122+
// COVERED_GENERIC or INTENTIONAL_SKIP_*.
120123
var acceptedMissingMethods = map[string]string{
121124
// COVERED_VARIANT — an alternative SDK binding for an endpoint another
122125
// covered method already drives; one action per endpoint is the rule.
@@ -164,11 +167,6 @@ var acceptedMissingMethods = map[string]string{
164167

165168
// COVERED_GENERIC — superseding/generic variant covers the same capability.
166169
"MergeRequests.GetMergeRequestApprovals": "COVERED_GENERIC. Mrapprovals config/state",
167-
"MergeRequests.GetMergeRequestChanges": "COVERED_GENERIC. gitlab_mr_changes_get (ListMergeRequestDiffs)",
168-
"ProjectMembers.ListProjectMembers": "COVERED_GENERIC. gitlab_project_members_list (ListAllProjectMembers)",
169-
"Groups.ListGroupMembers": "COVERED_GENERIC. gitlab_group_members_list (ListAllGroupMembers)",
170-
"Groups.ListSubGroups": "COVERED_GENERIC. gitlab_subgroups_list (ListDescendantGroups)",
171-
"Groups.DeleteGroupLDAPLink": "COVERED_GENERIC. Groupldap ForProvider/WithCNOrFilter variants",
172170
"PersonalAccessTokens.RotatePersonalAccessTokenByID": "COVERED_GENERIC. gitlab_personal_access_token_rotate",
173171
"ExternalStatusChecks.CreateExternalStatusCheck": coveredGenericDeprPrj,
174172
"ExternalStatusChecks.DeleteExternalStatusCheck": coveredGenericDeprPrj,
@@ -177,28 +175,9 @@ var acceptedMissingMethods = map[string]string{
177175
"ExternalStatusChecks.SetExternalStatusCheckStatus": "COVERED_GENERIC. Deprecated; SetProjectMergeRequestExternalStatusCheckStatus",
178176
"ExternalStatusChecks.UpdateExternalStatusCheck": coveredGenericDeprPrj,
179177
"ErrorTracking.EnableDisableErrorTracking": "COVERED_GENERIC. Deprecated; gitlab_enable_disable_error_tracking",
180-
"ErrorTracking.CreateErrorTrackingSettings": "COVERED_GENERIC. Same settings resource as UpdateErrorTrackingSettings",
181-
182-
// INTENTIONAL_SKIP_BINARY — raw file/archive/state bytes, unsuitable for JSON tools.
183-
"GroupMarkdownUploads.DownloadGroupMarkdownUploadByID": skipBinaryBytes,
184-
"GroupMarkdownUploads.DownloadGroupMarkdownUploadBySecretAndFilename": skipBinaryBytes,
185-
"ProjectMarkdownUploads.DownloadProjectMarkdownUploadByID": skipBinaryBytes,
186-
"ProjectMarkdownUploads.DownloadProjectMarkdownUploadBySecretAndFilename": skipBinaryBytes,
187-
"GroupRelationsExport.ExportDownload": "INTENTIONAL_SKIP_BINARY. Export archive bytes",
188-
"SecureFiles.DownloadSecureFile": "INTENTIONAL_SKIP_BINARY. Secure file bytes",
189-
"TerraformStates.Download": "INTENTIONAL_SKIP_BINARY. Tfstate bytes",
190-
"TerraformStates.DownloadLatest": "INTENTIONAL_SKIP_BINARY. Tfstate bytes",
191-
"Groups.DownloadAvatar": "INTENTIONAL_SKIP_BINARY. Group avatar image bytes (*bytes.Reader); upload IS exposed",
192178

193179
// INTENTIONAL_SKIP_OTHER
194-
"Jobs.GetJobTokensJob": "INTENTIONAL_SKIP_OTHER. CI-job-token self-lookup; not usable with a PAT",
195-
"Repositories.StreamArchive": "INTENTIONAL_SKIP_OTHER. Streaming dup of Repositories.Archive (gitlab_repository_archive)",
196-
// This used to read "COVERED_RAW. gitlab_mr_approval_config", and it was
197-
// the paper trail behind twenty phantom output fields: that action calls
198-
// the GET at this path and never the POST, but the claim of coverage made
199-
// the POST's 24-field response look like the shape the action should
200-
// publish. Set approval requirements through the approval rules instead.
201-
"MergeRequestApprovals.ChangeApprovalConfiguration": "INTENTIONAL_SKIP_OTHER. Deprecated in GitLab 16.0; approval rules supersede it (gitlab_mr_approval_rule_create/update)",
180+
"Jobs.GetJobTokensJob": "INTENTIONAL_SKIP_OTHER. CI-job-token self-lookup; not usable with a PAT",
202181
}
203182

204183
// AcceptedMissingMethods returns the adjudications acceptedMissingMethods

‎cmd/audit_1to1/internal/routes/analyze.go‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,30 @@ type Summary struct {
157157
SkipReasonsReached []string `json:"skip_reasons_reached"`
158158
}
159159

160-
// clean reports whether the gate passes: no declaration that no longer
161-
// describes the tree. A finding is a report, not a failure, until the
162-
// triage adjudicates them.
163-
func (s *Summary) clean() bool { return s.Stale == 0 }
160+
// clean reports whether the gate passes: no route left a finding, of any
161+
// class findingClasses names, and no declaration that no longer describes
162+
// the tree. A route no action reaches is answered by a rule, an inherited
163+
// decision or a family row, so a re-pin that brings one, or an action that
164+
// stops sending one, fails the gate until the change that causes it decides
165+
// the route.
166+
func (s *Summary) clean() bool {
167+
counters := s.counters()
168+
for class := range findingClasses {
169+
if *counters[class] != 0 {
170+
return false
171+
}
172+
}
173+
return s.Stale == 0
174+
}
164175

165176
// count adds one route of class to the summary.
166177
func (s *Summary) count(class string) {
167-
counters := map[string]*int{
178+
*s.counters()[class]++
179+
}
180+
181+
// counters names the counter of each class.
182+
func (s *Summary) counters() map[string]*int {
183+
return map[string]*int{
168184
ClassReached: &s.Reached,
169185
ClassOutsideAPI: &s.OutsideAPI,
170186
ClassSkip: &s.Skipped,
@@ -178,7 +194,6 @@ func (s *Summary) count(class string) {
178194
ClassInternalNotExcluded: &s.InternalNotExcluded,
179195
ClassContradicted: &s.Contradicted,
180196
}
181-
*counters[class]++
182197
}
183198

184199
// Options is what a run of this scope needs beyond the tree.

0 commit comments

Comments
 (0)