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.go adds opt-out, per-row opt-in (asserting the ObjectID is the upstream repository id, not the association id), malformed-parameter 400, empty-list opt-in driven through the real authz.VerdictEvaluator to 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 pins applyBatchCheckResults, a hand-rolled re-validation of raw []glaz.BatchCheckResult on the listing filter's glaz.BatchChecker seam, which this route never touches. This route goes through authz.VerdictEvaluator.Evaluate → correlateVerdicts, whose own six-case table in internal/authz/verdicts_test.go is pre-existing and shared by every consumer. The four cases here cover the three branches verdictBatchAnswered can additionally fail on.
  • verdict_batch_internal_test.go covers 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' shared strictBooleanParams vocabulary.

Guardrails

  • e2e catalog (13): adds e2e.npm.upstreams.list-verdicts to docs/testing/e2e/npm.md.
  • Bruno (15): list-upstream-repositories.bru gains the ~include_permissions query parameter alongside the OpenAPI change.
  • docs/user/** (23): no change. docs/user/ does not document include_permissions on 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)

Edited by David Fernandez

Merge request reports

Loading
Loading