feat(authz): add the npm RouteParser (S09 Enforcement plan: 11/20)

🎯 What

Adds internal/authz's npm RouteParser: npmRouteParser.Mine claims paths whose post-slug segment is the literal npm (derived from npmroute's own constants), Parse delegates to npmroute.ParseRoute and maps each route-and-method pair to its ADR-021 action — the action comes from the route, not the verb alone, so the -rev/{rev} unpublish PUT maps delete_artifact and the dist-tag verbs split create_artifact/delete_artifact. An unmapped method on a recognized route is RouteUnmapped (403) and an unparsed path under the mount is RouteUnrecognized (404), both returning the zero ParsedRoute. NewNpmMiddleware wires the parser into the shared Assembly with read_artifact as the masking follow-up action, mirroring NewMavenMiddleware. authz-format-boundary's allow list gains the npmroute leaf.

It also lands the escaped-path guard the plan's step 11 amendment assigns to this step. Go's ServeMux matches EscapedPath while the grammar matches the decoded path, so an encoded separator makes the two read one request as different routes — and on the -rev shapes, different actions. RouteParser.Parse now receives r.URL.EscapedPath() (the OCI and Maven parsers ignore it), and the npm parser rejects any %2F outside the single scope-separator position (one occurrence in a segment led by @ or its %40 spelling, the form the packument generator mints into tarball URLs) as the new RouteEscapedSeparator outcome: a masked 404 counted under its own denial_reason="escaped_separator" label rather than folded into unparsed_route.

Nothing wires the constructor in this MR. The holder, the adapter, the mux-registration wrap, and the route-table parity pin are step 13 of the plan, so no request behavior changes on any live path: the Parse signature widening is plumbing the live OCI path ignores, and the new denial reason is emitted by nothing reachable.

💭 Why

Step 11 of the S09 enforcement plan. The container surface proved the seam contract and Maven repeated it; npm supplies its parser here, consuming the npmroute leaf step 10 landed and the per-surface masking action step 5 parameterized. The guard must exist before step 13 makes npm enforce: from that point the authorization decision and the audit record would otherwise name a delete for an operation that creates, which becomes a privilege escalation the day a delete-only role can exist.

🔍 Design notes

  • The guard spans the whole escaped path, not just the package-name capture. That is what closes every escaped divergence shape npmroute's route_disagreements.md enumerates — the capture swap, the target swap, the file-name swap, and the reverse family this MR documents (a mux-side unpublish the grammar reads as a dist-tag create_artifact) — while a capture that agrees on the single scope separator is not a mismatch and passes. The escape-free rejoin swap carries nothing to reject and is action-safe today; https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/950 tracks the by-construction seam fix that would close it too.
  • Decoded-path containment is stated and pinned, not assumed. Mine reads the decoded r.URL.Path, so a slug holding an encoded slash either defeats Mine (pass-through, then the dispatcher 404s on the slug charset) or is now rejected by the guard. TestNpmRouteParser_MineCoversParseRoute pins the Mine ⊇ ParseRoute superset property with a vacuous-pass guard.
  • RouteUnmapped before the dispatcher's 405, matching shipped OCI and Maven behavior per S09 (authorization).
  • npmAction's switch is exhaustive over npmroute.Route, RouteUnknown and out-of-range values fail closed, and the out-of-range row is mutation-verified: granting an action from the default arm fails the suite.

⚠️ Plan drift

  • The plan's Files: list named internal/authz/npm.go, .golangci.yaml, and npm_test.go. The guard adds middleware.go (the Parse signature and the routeStatus arm), oci.go/maven.go (ignored-parameter ripple), metrics.go and internal/metrics/cardinality.go (the escaped_separator reason, budget 6→7), and npm_internal_test.go beside the named test file.
  • Review rounds corrected prose this import falsifies: route_disagreements.md's reachability and guard-ownership paragraphs (and its "only shape with different actions" claim, false against the reverse family), the npmroute-stays-a-leaf comments' tense, and the depguard:negative-control job's cycle description in .gitlab-ci.yml, which now names the real closing hop (npmrules' shared-tag probe importing internal/format/oci).
  • maven_internal_test.go gains a //nolint:dupl (measured, not reasoned: dupl fires on the mirrored Mine tables the moment the npm file exists).

🧪 Testing

Plan acceptance criterion Covered by
Each row of the spec's table yields its action TestNpmAction full matrix; TestNpmRouteParser_Parse_RouteMatched, one row per spec route plus scoped variants
-rev/{rev} PUT yields delete_artifact, not create_artifact TestNpmAction; TestNpmRouteParser_Parse_RouteMatched
Dist-tag PUT yields create_artifact, DELETE delete_artifact TestNpmAction; the masking row in TestFormatMiddleware_MaskingReadAction uses the dist-tag DELETE
HEAD maps wherever GET does TestNpmAction HEAD rows, plus HEAD-unmapped rows on the write-only routes
Unmapped method on a recognized route yields RouteUnmapped TestNpmAction mismatch rows (PATCH/OPTIONS/TRACE/CONNECT included); TestNpmMiddleware_ScopeAndFailClosed DELETE/POST/PATCH → 403
Guard (step 11 amendment): %2F outside the scope separator denies masked 404 under its own denial_reason TestNpmRouteParser_Parse_EscapedSeparator (both cases of the hex, reverse family, target and file-name swaps, double-separator behind either scope spelling, allowed scoped forms in both the @ and %40 spellings); TestNpmMiddleware_EscapedSeparatorDeniesBeforeCheck; the extended routeStatus reason assertions

go test ./... green, golangci-lint run 0 issues (default and --build-tags=integration invocations), lint:comment-caps passes at the tip, and the depguard:negative-control probes still fire under the widened allow list.

e2e scenario catalogs: unchanged, deliberately. The parser is reachable from no request path in this MR; the Access control rows in docs/testing/e2e/npm.md move off blocked in step 13, the step that makes npm enforce.

📏 Diff size

+930/−71 across 17 files, past the 500-line justification threshold: 214 lines of production Go (190 of them the one new npm.go; the rest the Parse signature ripple and the metrics constant), 624 of tests, and 92 of config and docs (comment corrections and the divergence doc). Splitting the guard from the parser would leave the plan-amended acceptance half-met on main, and splitting the tests from the tables they pin would raise the count a reviewer holds in mind rather than lower it.

Related to #853 (closed)

Edited by David Fernandez

Merge request reports

Loading
Loading