feat(npm): add the npmroute leaf grammar and its depguard fence (S09 Enforcement plan: 10/20)

🎯 What

Adds internal/format/npm/npmroute, the dependency-free npm mount grammar: ParseRoute parses the decoded URL path /{slug}/npm/{repository_name}/<remainder> against the S09 npm route table, returning a closed Route enumeration plus the captures each route carries (PackageName, FileName, Tag, Rev), and rejoining a scoped package name (@scope/name, arriving decoded from @scope%2Fname) at the package-name position of the row being matched. Its leaf-ness is fenced by a new npmroute-stays-a-leaf depguard rule and a negative-control probe under the existing depguard_nc_ociroute build tag, wired into the depguard:negative-control CI job.

💭 Why

Step 10 of the S09 enforcement plan. internal/authz cannot import internal/format/npm without closing a cycle through internal/server, so the route grammar the npm RouteParser needs lives in a fenced leaf package — the shape internal/format/oci/ociroute proved for the container surface and internal/format/maven/mavenroute repeated for Maven.

Nothing imports the package in this MR. The RouteParser that consumes it and the wiring that mounts it are steps 11 and 13 of the plan.

🔍 Design notes

  • The enumeration has ten values, one per distinct path shape; the plan says "the thirteen routes". Thirteen counts the spec table's (row, path) pairs without deduplicating same-path rows that differ only by method. The grammar is method-agnostic — the authz npm RouteParser (step 11) maps (route, method) to the ADR-021 action, exactly how the spec table is keyed — so deduplication by path shape is correct and no route is dropped: each of the dispatcher's 14 mux.Handle registrations maps onto one of the ten shapes.
  • The rejoin applies at the package-name position of the row being matched, not only at the remainder's head. The spec's "a leading @-prefixed segment" reads literally only for the /{package_name}... rows; the dist-tags rows carry the scoped name after /-/package/, and the acceptance criterion (scoped variants of every package-name route) requires the rejoin there too. An npm package name carries no slash other than the scope separator, so the rejoin is unambiguous.
  • Captures are unvalidated. The package does not interpret what it captures; npm name, version, and file-name rules stay in internal/format/npm/npmrules. Decoding is the caller's obligation, and the tables pin why rather than the reverse: an escaped scope parses only when a spare segment follows it to rejoin against, so the undecoded scoped shapes a real client sends are rejected, not kept verbatim. A failure row covers the bare undecoded scoped name.
  • The grammar and the dispatcher's mux deliberately diverge, in four shapes. The mux matches each wildcard as one escaped segment; this grammar matches decoded segments and rejoins. internal/format/npm/npmroute/route_disagreements.md states that single cause and enumerates what it produces: the capture swap that changes the action, the rejoin swap that needs no percent-encoding at all, the target swap that names a different repository, and the file-name swap. Only the first changes the ADR-021 action, and it is unreachable today (nothing imports the package) and unexploitable today (every built-in role holding delete_artifact also holds create_artifact, and custom roles are deferred from closed beta); it becomes a privilege escalation the day a delete-only role can exist. The target swap is contained by the slug and repository charsets rather than by the grammar, so that containment lapses if nested slugs ship. Closing them belongs to step 11's parser, and the two options are not equivalent: splitting EscapedPath() per segment closes all four, while rejecting a stray %2F leaves the escape-free rejoin swap open.
  • The probe imports internal/format/maven/mavenroute rather than internal/format/npm/npmrules. npmrules sits under npmroute's own parent, and a fence that ever carved out same-family imports would stop denying it without the probe noticing; mavenroute is itself a fenced leaf that never reaches server or authz, so the import closes no compile cycle under the tag.
  • The remainder split is bounded. ParseRoute splits the remainder with SplitN at eight elements rather than an unbounded Split, so path length cannot drive allocation once step 11 calls it ahead of the anonymous gate. The longest accepted row is six segments, so the bound moves no verdict: seven or more already fell to the default reject.

🤝 Obligation recorded in the plan

The step 11 guard obligation and its merge order are recorded in the S09 plan by !1972 (merged), the single-writer docs(plans) branch, in commits 189e8982 and 91c2d3f3. Guardrail 4 keeps the plan edit out of this MR, so while !1972 (merged) is open the obligation lives only there and in this MR's review threads; it must land for the plan to carry it. It amends step 11's Depends on line (the disagreement, the two closing options and why they are not interchangeable, and the requirement that the guard precede step 13), adds step 11's acceptance clause that the guard's rejections carry their own denial_reason, and corrects step 13's parity wording. Neither MR changes code the other depends on, so the two are unordered.

⚠️ Plan drift

One addition beyond Step 10's Files: list: FuzzParseRoute and its fuzz:npm line. The fuzz job's package arguments do not recurse, so the new leaf package needed its own entry; without it the target would only ever replay its seeds. fuzz:oci and fuzz:maven carry the exact analogue for their grammar leaves.

🧪 Testing

Plan acceptance criterion Covered by
Every route of the spec's table resolves TestParseRoute_SuccessTable — 19 rows over the ten routes, including the disambiguation rows (packages named -, -rev, npm; slug and repository named npm; no-leading-slash form; an escaped scope with a spare segment keeping its %2F verbatim)
A scoped request and its %2F-escaped tarball equivalent resolve identically to their unscoped forms TestParseRoute_ScopedPackage — one row per package-name-carrying route, each asserting the full ParsedRoute for the pre-split scoped path and its escaped-then-decoded equivalent
A path under the mount matching no route reports no match TestParseRoute_FailureTable — 35 rows, each row's comment naming the guard that rejects it, each asserting the zero-value return
The depguard:negative-control job reports npmroute-stays-a-leaf as fired The job's rule loop gains the rule; reproduced locally, the depguard_nc_ociroute invocation fires npmroute-stays-a-leaf (alongside format-isolation on the same import line, which the job's --uniq-by-line=false keeps visible) with no typecheck or load failure

TestParseRoute_DisagreesWithTheMux pins all four shapes across five rows, including the mux-hit / grammar-miss case the spec calls breaking the route silently for every caller. Neither is reachable from FuzzParseRoutecheckParseRouteInvariants rebuilds the path from the grammar's own capture shape, so the round trip stays self-consistent through a disagreement the mux is not party to.

FuzzParseRoute (seeded from the three tables) asserts non-empty mount fields, per-route capture presence and absence, and a byte-exact round-trip reassembly of every accepted input; multi-minute local runs found nothing. Statement coverage on the package is 100%, mutation-checked: narrowing either rejoin guard or dropping a literal check fails a named row. go test ./... green, golangci-lint run 0 issues, and the comment-caps check passes at the tip.

e2e scenario catalogs: unchanged, deliberately. The package 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

+1198/−10 across 6 files, past the 500-line justification threshold. 747 of the added lines are the test suite, 104 the disagreement sidecar and 12 the probe, with the .golangci.yaml rule and its .gitlab-ci.yml wiring accounting for most of the remainder, leaving 241 lines of new production Go in one file. The plan's Step 10 records the seam interrogation: the rejoin is consumed by every route, so a step shipping the table without it would deny every scoped request until its successor landed, and splitting the grammar from the table that pins it or the probe from the rule it exercises would raise the count a reviewer has to hold in mind rather than lower it.

🔁 Known duplication, handled in a follow-up

The grammar duplicates route parsing the npm dispatcher already does with its mux patterns — the seam contract's deliberate trade-off, since internal/authz cannot import the dispatcher. Keeping the two from drifting is tracked in #392 alongside the request-time duplication it already records; the route-table parity test lands with the npm wiring step.

Related to #852 (closed)

Edited by David Fernandez

Merge request reports

Loading
Loading