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'sroute_disagreements.mdenumerates — 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-tagcreate_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.
Minereads the decodedr.URL.Path, so a slug holding an encoded slash either defeatsMine(pass-through, then the dispatcher 404s on the slug charset) or is now rejected by the guard.TestNpmRouteParser_MineCoversParseRoutepins theMine ⊇ ParseRoutesuperset property with a vacuous-pass guard. RouteUnmappedbefore the dispatcher's 405, matching shipped OCI and Maven behavior per S09 (authorization).npmAction's switch is exhaustive overnpmroute.Route,RouteUnknownand out-of-range values fail closed, and the out-of-range row is mutation-verified: granting an action from thedefaultarm fails the suite.
⚠️ Plan drift
- The plan's
Files:list namedinternal/authz/npm.go,.golangci.yaml, andnpm_test.go. The guard addsmiddleware.go(theParsesignature and therouteStatusarm),oci.go/maven.go(ignored-parameter ripple),metrics.goandinternal/metrics/cardinality.go(theescaped_separatorreason, budget 6→7), andnpm_internal_test.gobeside 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), thenpmroute-stays-a-leafcomments' tense, and thedepguard:negative-controljob's cycle description in.gitlab-ci.yml, which now names the real closing hop (npmrules' shared-tag probe importinginternal/format/oci). maven_internal_test.gogains a//nolint:dupl(measured, not reasoned:duplfires on the mirroredMinetables 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)