feat(managementapi): embed upstream repository verdicts in the npm list
What this does
Adds the existing include_permissions opt-in to the npm listUpstreamRepositories route (GET .../npm/upstream_repositories). The list handler is shared with the Maven arm, which reads no include_permissions and behaves as it does on main; Maven verdicts are a follow-up. On opt-in, one GLAZ BatchCheck evaluates authz.RepositoryVerdictActions() against each row's upstream repository, and the result rides on each row's upstream_repository as a permissions object, so the UI can gate its per-upstream controls without a request per row.
This makes the npm upstream list the fourth verdict-embedding surface, after repository detail, repository list, and namespace details.
Without the parameter the response is byte-for-byte what it was: a bare array, no permissions key anywhere.
{
"id": "…f0c7…",
"position": 1,
"upstream_repository": {
"id": "…cb87…",
"name": "npm-upstream-a",
"format": "npm",
"kind": "hosted",
"permissions": {
"read_repository": true, "update_repository": true, "delete_repository": true,
"create_repository_upstream": true, "update_repository_upstream": true,
"delete_repository_upstream": true,
"read_artifact": true, "create_artifact": true, "delete_artifact": true
}
}
}The verdict is keyed on the upstream repository's own id, not the association row's — those are two different UUIDs on the same row, and confusing them would attach one repository's permissions to another.
Commits
| Commit | What |
|---|---|
| 42e64a3d | refactor(managementapi): converge the three existing verdict surfaces onto one evaluateVerdictBatch helper |
| 6cd794ae | feat(managementapi): the npm upstream list verdicts themselves |
| 6f1e99e3 | docs(managementapi): restore rationale the refactor's comment compression dropped |
| a02ce769 | docs(specs): the S09 acceptance criterion for the new surface |
| e27e0b97 | fix(managementapi): name the missing action a shape mismatch's delivered lengths hide |
| 828a98b8 | fix(managementapi): read include_permissions on the npm arm only, since the upstream list handler is shared with Maven |
The refactor lands first on purpose. detail.go, list.go, and namespace_detail.go each carried their own copy of the Evaluate → renderDecision → shape-check → writeContractViolation sequence; this change would have added a fourth. The helper is format-agnostic, and the Maven and Container/OCI upstream arms are already declared in the contract, so the seam is on its way to carrying six surfaces.
One behavior-visible consequence of the refactor: namespace_detail.go had two bespoke contract-violation log messages, which converge to the shared diagnostic every other surface already emits. Review raised that the delivered lengths alone cannot distinguish a swapped action from a healthy batch, since that case keeps len(set) == len(actions), so e27e0b97 gives the shared check a reason string: it names the failing query index and, for the equal-size case, the missing action and the unrequested one delivered in its place. All four surfaces gain that detail, which is more than namespace_detail.go carried before the refactor. That changes the logged text on the two already-live endpoints too: detail.go and list.go keep their pre-refactor message strings, but the helper now appends : <mismatch> to them, so an exact-match search or alert on the old strings for repository detail or repository list stops matching. A prefix match still does. The 500 response envelope is unchanged. Status codes, fail-closed posture, and short-circuits are unchanged on all three migrated surfaces.
Why no plan file
The work item was graded NO PLAN and both the plan-MR and spec-MR gates were waived for it. The S09 amendment rides in this MR rather than preceding it, which is what the work item's own out-of-scope note prescribes.
Governing ADR — read the handbook, not the mirror
ADR-021 and ADR-009 were amended in gitlab-com/content-sites/handbook!21209 (merged) to sanction exactly this shape. The local docs/adr/ mirror has not synced that amendment yet (scripts/adr-freshness.sh reports it as one of three unsynced commits), so docs/adr/021_authorization.md in this tree still shows pre-amendment text. Review against the handbook version.
The amendment settles the two questions this change raises:
- Existence disclosure — "a virtual repository's upstream list names the sibling repositories it resolves through with or without verdicts, so embedding their verdicts discloses no resource the list did not." The row set is never changed by the opt-in: a repository the principal cannot read stays listed, with deny on every action.
- Cost — "an upstream list's verdicts are one BatchCheck per page", bounded by ADR-004's 20-upstream cap. Worst case is 20 items × 9 actions = 180 pairs, well inside GLAZ's limits.
A sub-resource list gets no by-product namespace evaluation, unlike the repository list: row queries only.
Size
819 reviewable lines, past the 500 mark in guardrail 17, so here is the split:
| Group | Added | Deleted |
|---|---|---|
Production Go (upstream_repositories.go, new verdict_batch.go, the three migrated surfaces) |
174 | 116 |
Tests (npm_virtual_upstreams_test.go 289, verdict_batch_internal_test.go 152, verdict_contract_test.go 54) |
496 | 1 |
| Contract and docs (OpenAPI, Bruno, S09, S17, e2e catalog) | 24 | 8 |
71% of the added lines are tests, and the production half is a net +58. I did not split it: separating the refactor from its only new consumer would leave a reviewer judging a shared helper with nothing using it, which is harder to review rather than easier.
Testing
npm_virtual_upstreams_test.goadds opt-out, per-row opt-in (asserting theObjectIDis the upstream repository id, not the association id), malformed-parameter 400, empty-list opt-in driven through the realauthz.VerdictEvaluatorto exercise the zero-query short-circuit end to end, evaluator failure (503 and abandon), and a four-case malformed-shape table.- The malformed-shape table is four cases, not the six of
list_authz_test.go, on purpose. That six-case table pinsapplyBatchCheckResults, a hand-rolled re-validation of raw[]glaz.BatchCheckResulton the listing filter'sglaz.BatchCheckerseam, which this route never touches. This route goes throughauthz.VerdictEvaluator.Evaluate→correlateVerdicts, whose own six-case table ininternal/authz/verdicts_test.gois pre-existing and shared by every consumer. The four cases here cover the three branchesverdictBatchAnsweredcan additionally fail on. verdict_batch_internal_test.gocovers the new helper directly: denial, abandon, shape mismatch, matching shape, and the two diagnostic shapes e27e0b97 added (size mismatch, swapped action).- Two new contract tests pin the OpenAPI operation's opt-in and the summary's optional
permissions. - Verified against a running instance, not only the suite: a virtual npm repository with two hosted upstreams, driven over HTTP. Opt-out and opt-in confirmed live, along with the row set being identical across both. Malformed values — empty, repeated, and uppercase
TRUE— all answer 400 rather than coercing, matching the other three surfaces' sharedstrictBooleanParamsvocabulary.
Guardrails
- e2e catalog (13): adds
e2e.npm.upstreams.list-verdictstodocs/testing/e2e/npm.md. - Bruno (15):
list-upstream-repositories.brugains the~include_permissionsquery parameter alongside the OpenAPI change. docs/user/**(23): no change.docs/user/does not documentinclude_permissionson any of the three existing surfaces, so there is nothing for a fourth to join. If that tree ever gains a permissions section, all four surfaces belong in it together.- Conformance (12): not affected. This is management-API authorization metadata, not npm wire-protocol behavior.
- Specs: S09 gains the fourth surface throughout and a new acceptance criterion; S17 updates both virtual-upstream-association contract tables. The new S09 criterion is appended as 30 rather than filed with the verdict group at 21–27, because criteria 28 and 29 are cited by number from two plan files and inserting would renumber them.
Related to #1174 (closed)