fix(elpx): converge the three entry-path normalizers on validate-never-rewrite

[erseco]

The .elpx entry-path rule had three implementations — PHP, TypeScript, and the Service Worker's inline mirror — and they did not agree. This converges them and records the decision as ADR-96-01.

What diverged

Measured against origin/main at 7520972, on PHP 8.5.9 and Node 26.6.0:

Input	PHP	TS / SW
a/b/c	a/b/c	a/b/c
../escape	null	null
a/b/../c	null	a/c
a/./b	null	a/b
a//b	a//b	a/b

This is a consistency fix, not a traversal fix. Every implementation rejected ../escape and none escaped the package root. The symptom was that a package containing a/b/../c rendered in the browser but 404'd from the PHP AssetController and the preview provider — and the docblock on normalizeEntryPath claimed it "matches the rule used by the PHP-side ZipEntryService", which was false.

What was adopted, and why

Reject, and rewrite nothing. An entry path is accepted only when it is already canonical, and it comes back byte-identical. Rejected: the empty string, any NUL byte, any backslash, and any empty, . or .. segment — which covers leading, doubled and trailing slashes. normalizeEntry(x) is either x or null, never a third string.

Resolving was the tempting option — more permissive, keeps working what works today — but it does not actually converge anything:

1. Entry names are looked up verbatim. ZipArchive::statName() matches central-directory names byte-for-byte, and the browser keys its map by the stored name. Against an archive built with four literal names:

   central directory names:
     [0] "a/b/../c"      [1] "a/c"      [2] "a/./d"      [3] "a//e"
   lookups:
     statName("a/b/../c") => found, content=DOTSEG
     statName("a/c"     ) => found, content=PLAIN
     statName("a/./d"   ) => found, content=DOTCUR
     statName("a/d"     ) => FALSE
     statName("a//e"    ) => found, content=DBLSLASH
     statName("a/e"     ) => FALSE
   

   a/b/../c and a/c are two different entries with different contents in one archive. A resolving normalizer asked for the first serves the second. Resolving in PHP would not even fix the reported 404 — the route would look up a/c, which is a different entry or absent.

2. Dot-segment entries are unaddressable anyway. URL parsers apply RFC 3986 §5.2.4 dot-segment removal before a request is dispatched, and percent-encoding does not evade it:

   "/base/a/./d"      -> pathname "/base/a/d"
   "/base/a/b/../c"   -> pathname "/base/a/c"
   "/base/a/%2E%2E/c" -> pathname "/base/c"
   encodeURIComponent(".") = "."   encodeURIComponent("..") = ".."
   

   Both the SW route and the asset route address entries through a URL path, so an entry whose stored name contains a dot segment can never be requested, whatever the normalizer decides.

Rejecting also makes the rule one sentence long, which is what matters when reviewing security code: the accepted set is a predicate and the return value is the input, so the three implementations agree by construction.

What could break

• A package containing a non-canonical entry name now fails to open entirely (ZipReadError / UNSAFE_ENTRY) where the browser previously rendered it. Every .elp and .elpx available locally — this repo's fixtures plus the editor's test corpus — was scanned: 224 packages, 43 539 entries, 0 non-canonical names. JSZip, Archiver and Python's zipfile all emit forward-slash relative names, and the ZIP spec requires it (APPNOTE §4.4.17.1: a stored name "MUST NOT contain a drive or device letter, or a leading slash", and "All slashes MUST be forward slashes").
• Entry names legal on POSIX but not in the ZIP format — a filename containing a backslash, most plausibly — are refused. Refusing is safe; mis-serving is not.
• readEntry() no longer tolerates a leading slash on a caller-supplied name. All in-tree callers pass canonical names.
• resolveRelativeEntry still resolves ./ and ../, because an href inside package HTML may legitimately contain them: it resolves first (RFC 3986) and validates the result. A leading slash in an href now addresses the package root instead of producing a doubled slash that the old normalizer silently collapsed.

The shared vector table

tests/fixtures/entry-path-vectors.json is one file loaded by both suites — tests/js/paths.test.ts and tests/Unit/Service/ZipEntryServiceTest.php (via #[DataProvider]). No build step. The JS suite also evaluates the shipped src/sw/exelearning-sw.js in a node:vm context with a stub self, so the mirror is tested rather than transcribed; the mirror stays inline, as it must.

Input	Result
index.html	index.html	canonical root entry
a/b/c	a/b/c	canonical nested entry
html/page-1.html	html/page-1.html	canonical entry with punctuation
content/Página 1.html	unchanged	non-ASCII and spaces are ordinary characters
..../x	..../x	four dots is a name, not a dot segment
../escape	null	climbs above the package root
html/../../etc	null	climbs above the root after one level
a/b/../c	null	interior .. segment
a/b/..	null	trailing .. segment
a/./b	null	interior . segment
./a	null	leading . segment
.	null	bare .
..	null	bare ..
a//b	null	empty interior segment
....//x	null	empty interior segment behind a four-dot name
/leading	null	absolute path
/	null	nothing but a separator
//	null	nothing but separators
trailing/	null	directory entry
a\b	null	backslash
\leading	null	Windows-style absolute path
(empty string)	null	empty string
a<NUL>b	null	NUL byte
<NUL>	null	bare NUL byte

Reverting just the empty-segment check in the Service Worker fails 6 of these, so the guardrail detects a real divergence rather than a transcribed one.

Verification

Run locally on macOS, PHP 8.5.9, Node 26.6.0:

npm run typecheck        exit=0
npm test                 exit=0    Test Files 6 passed (6) · Tests 108 passed (108)
vendor/bin/phpunit --configuration tests/phpunit.xml
                                   OK (37 tests, 75 assertions)
make architecture-check  exit=0    Architecture records OK — 2 records, 0 changes.
make lint                exit=0
npm run lint:biome       exit=0    10 warnings, all pre-existing (7 in biome.json, new-menu.ts:58, exelearning-sw.js:125)
vendor/bin/php-cs-fixer fix --dry-run --diff --using-cache=no
                         exit=0    Found 0 of 17 files that can be fixed
npm run build            exit=0    webpack compiled successfully
git diff --check         exit=0

Not verified locally: the Nextcloud server matrix (CI boots a real server; nothing here touches DI, routes or info.xml).

ADR

docs/architecture/adr/ADR-96-01-validate-entry-paths-instead-of-rewriting-them.md — status: Proposed. .agents/skills/elpx-package-safety/SKILL.md documented the divergence as an open defect; it now documents the converged rule and points at the ADR.

[github-actions]

Preview this PR in the Nextcloud Playground

A fresh Nextcloud boots in your browser with this branch's exelearning app installed and enabled (log in as admin / admin). Two sample .elpx are seeded under exelearning-samples/ in Files — click one to open the viewer.

eXeLearning editor: v4.0.2 (overlaid at boot from the upstream release).