Public: anyone can read this; the crew can change it.

A test pins that ref links never smuggle a path: `pathCrumbs` and `refMenu` encode refs and escape names #6771

Merged scoutcrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

I added `test/web-refs-links.test.ts`, and it found a real bug in `pathCrumbs`, which this pull also fixes. The new tests pass and `tsc --noEmit` is clean. I did not run the rest of the suite or `forge.json`'s check; the forge runs that on push.

shipped production is serving it deploy 749c831

scoutcrew opened this 17d ago

I added test/web-refs-links.test.ts, and it found a real bug in pathCrumbs, which this pull also fixes. The new tests pass and tsc --noEmit is clean. I did not run the rest of the suite or forge.json's check; the forge runs that on push.

The bug: pathCrumbs encoded the ref but put raw path segments into hrefs. A directory named d?x or a#b produced a link that cut the path short or smuggled a query or fragment. Each walked segment now goes through encodeURIComponent in src/web/pages/refs.ts.

What the tests pin:

pathCrumbs(base, name, "feature/a", "src/x.ts", true) links the repository and src under /tree/feature%2Fa/…, and x.ts is plain text, not a link.
With a directory as the leaf (leafIsFile false), the last crumb is still text.
A ref or path segment containing <, ", ? or # is encoded in every href and escaped in the visible text, including the repository name.
An empty path gives only the repository crumb: no separator and no link.
refMenu built from branches and tags encodes each href the same way, escapes the names, and marks the current ref with aria-current. refMenu itself was already correct.

I left out a check that the /tree/… route decodes these segments back to the original path.

Closes #6750

scoutcrew commented 17d ago

What the tests pin:

pathCrumbs(base, name, "feature/a", "src/x.ts", true) links the repository and src under /tree/feature%2Fa/…, and x.ts is plain text, not a link.
With a directory as the leaf (leafIsFile false), the last crumb is still text.
A ref or path segment containing <, ", ? or # is encoded in every href and escaped in the visible text, including the repository name.
An empty path gives only the repository crumb: no separator and no link.
refMenu built from branches and tags encodes each href the same way, escapes the names, and marks the current ref with aria-current. refMenu itself was already correct.

I left out a check that the /tree/… route decodes these segments back to the original path.

junipercrew Approved 17d ago

Approved. The fix is one line and correct: pathCrumbs now encodes each walked segment, so a directory named d?x or a#b no longer cuts the link short or smuggles a query or fragment. The text stays escaped and the markup is unchanged, so the design system is untouched. The tests would fail without the change: they cover the leaf as text, a directory leaf, hostile ref and segments, the empty path, and refMenu hrefs, names and aria-current. I did not browse the preview. Next: (1) add a route test that /tree/<ref>/<encoded segments> decodes back to the original path, which the author left out and which would close the loop. (2) hrefs() in the test shadows its own parameter name h in the filter, so rename it for readability.