Repository navigation
Harden Cutline imports and test portability - #90
Merged
MyPrototypeWhat merged 1 commit intoOct 27, 2026
Merged
Conversation
MyPrototypeWhat
approved these changes
Oct 27, 2026
MyPrototypeWhat
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at 17931d6. Thanks for the follow-up to #76. Nothing needs to change before merge. Approving.
Checks run
npx -y -p node@22 npm run checkat the repository root passes.- Plugin tests (
node --test tests/*.test.mjs) with FFmpeg on PATH: 190 tests, 177 pass, 0 fail, 13 skipped (browser-only tests, no Chromium on the machine). - Same suite with only Node and Git on PATH, from a clean fixture folder: 144 pass, 0 fail, 46 skipped. The media tests skip with a clear reason instead of failing.
Verified
- Single-file import and
fablecut_analyze_referencenow resolve the path withrealpathbefore the extension check, and single-file import copies from the resolved path. A.mp4symlink pointing at a.txtfile is refused by both, and nothing lands inmedia/. - Extension checks are lower-cased. The
isFile()check rejects directories, including symlinks to directories. copyToMediare-checks the extension of the source it actually copies, so the two layers agree.paths.jsstill throws whenFABLECUT_DATA_DIRis unset. The header comment now matches that behavior, andtests/paths.test.mjscovers it.- The private-redirect test listens on
127.0.0.1while the redirect still names127.0.0.2. The request stub still proves no request is sent, so the test keeps its meaning and no longer needs a second loopback address on macOS. - Everything stays inside
plugins/klarkxy/cutline/. Repository CI,.github/, root scripts and the roottest/folder are untouched.
Optional
- The new symlink test fails on a second run in the same checkout. The fixture folder under
.fablecut-acceptance/persists, sosymlinkSyncthrowsEEXIST. Removingdisguised.mp4(for example withrmSync(link, { force: true })) before creating the link would make reruns pass. - The resolved path is checked and then copied in two separate steps, so a process that can already write to the source folder could swap a path component in between. That attacker already has local write access, and the copy still lands only in
media/, so this is defense in depth. Opening the file once and checkingfstaton the handle would close the gap if you want it. - The copy is named after the link (
path.basename(src)), not the target. If a.pnglink points at a.mp4file, the copy keeps the.pngname with video content. Both are media, so this is harmless, but naming the copy after the resolved file would keep the extension consistent with its content.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FABLECUT_DATA_DIRmandatory, document the fail-fast behavior, and add a regression test for loading without it.127.0.0.1while retaining the127.0.0.2target and the no-request assertions.Validation
npm --prefix plugins/klarkxy/cutline test: 188 passed, 0 failed, 2 skipped (Windows symlink permission).PATH: 167 passed, 0 failed, 23 skipped (21 expected FFmpeg skips plus 2 Windows symlink skips).node --test plugins/klarkxy/cutline/tests/import-url-security.test.mjs: 8 passed.npm run validate: Cutline: OK;git diff --checkpassed.Environment notes
npm run checkvalidation passes, but two existing repository symlink tests fail locally with WindowsEPERM; the other 67 repository tests pass.127.0.0.2.Need help on this PR? Tag
@codesmith-botwith what you need.