Repository navigation
fix(preview): keep the home directory out of preview.html paths - #8
Conversation
An upward relative path from an output directory outside the home climbs to the root and back down through /Users/<name>/..., leaking the username even though every segment is relative. displayPath now prefers the tilde form in that case, and a thumbnail link that could only resolve by spelling out the home directory renders as plain text instead of a link. Also rewrites the README around a single options table, collapses the long reference sections, and adds the Top Shelf screenshot.
There was a problem hiding this comment.
🟡 Changes recommended
The updated path/link logic still uses a brittle rel.startsWith("..") check that misclassifies directories whose names begin with .., potentially reintroducing absolute/tilde paths unexpectedly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR fixes a privacy leak in generated preview.html by preventing paths/links that would reveal the user’s home directory (and thus username), and updates documentation accordingly.
Changes:
- Update
displayPath()to prefer~/...over upward-relative paths when the target is under the home directory but the reference directory is not. - Update preview thumbnail linking (
fileHref()) to suppress links that would require naming the home directory. - Add/adjust Jest coverage for upward-relative and preview rendering cases; refactor README into a consolidated options table and collapsible sections.
File summaries
| File | Description |
|---|---|
| tests/utils/paths.test.ts | Adds regression coverage for upward-relative paths that would otherwise spell out /Users/<name>/.... |
| tests/generators/preview.test.ts | Verifies rendered previews never contain homedir() and that home-resident inputs render unlinked. |
| src/utils/paths.ts | Adjusts displayPath() to avoid upward-relative home leaks by preferring tilde form in the leaking case. |
| src/generators/preview.ts | Adjusts fileHref() to avoid generating links that would expose the home directory. |
| README.md | Documentation reorganization and option-surface parity table consolidation. |
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
`startsWith("..")` also matches a directory legitimately named `..old`, so a
file inside the output directory was read as an escape: displayPath fell back
to the absolute form under zip output, and fileHref dropped the thumbnail link
outright. Both call sites now share isUpward().
There was a problem hiding this comment.
🟢 Ready to approve
The changes directly address the described leak scenarios, keep behavior consistent in safe cases, and are backed by targeted new tests for both path utilities and generated preview output.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Released as v1.4.1 and published to npm. |
The bug
preview.htmlcould spell out the home directory, and therefore the username, in a page meant to be committed or handed to a designer.Two paths reached it:
displayPath()withallowUpward. From an output directory outside the home into a source inside it,relative()climbs to the root and back down through/Users/<name>/.... Every segment is relative, so the existing tilde guard never fired.fileHref()thumbnail links. The absolute style (zip output) always emitted the real path, and the relative style hit the same upward-climb case.The fix
displayPath()prefers the tilde form whenever the target is under the home and the reference directory is not.fileHref()returns an empty href when a link could only resolve by naming the home directory.linked()already degrades to plain text, so those thumbnails render unlinked rather than leaking. Links that stay inside the output directory are unchanged.Tests
tests/utils/paths.test.ts: upward path from a temp dir into the home resolves to~/...; two paths both outside the home still stay upward.tests/generators/preview.test.ts: asserts the rendered page never containshomedir()in either link style, and that home-resident inputs render unlinked.306 tests pass,
npm run buildclean.Also in this PR
README pass: every option consolidated into one table (CLI flag / config key / plugin prop / type / default), the option-parity, generated-files, full-config, troubleshooting and tarball sections collapsed, old preview screenshots dropped, and a new Top Shelf screenshot as the hero. 859 lines to 583.
Release
Labeled
version:patch, so merging publishes v1.4.1.