fix: resolve package root by directory depth in clone command#309
Open
cs-raj wants to merge 1 commit into
Open
fix: resolve package root by directory depth in clone command#309cs-raj wants to merge 1 commit into
cs-raj wants to merge 1 commit into
Conversation
The clone command computed its content directory path by substring- splitting __dirname on '/src/' or '/lib/'. In a published/installed layout this breaks because the install path itself can contain '/lib/' (e.g. Node's own lib/node_modules), so the split matched the wrong occurrence and resolved to an ancestor directory instead of the package root. As a result, the pre-clone check, the post-import cleanup, and the interrupt-handler cleanup were all deleting an unrelated, effectively empty directory, while the actual export/import data (written to the correct path via clone-handler.ts's own resolution) was never cleaned up. Content from a previous clone run persisted on disk and bled into subsequent clones of a different stack. Resolve packageRoot by fixed relative directory depth instead, matching the approach already used in clone-handler.ts. This is independent of the surrounding install path and has been verified against nested, flat/hoisted, and pnpm virtual-store install layouts.
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
naman-contentstack
approved these changes
Jul 24, 2026
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
The
cm:stacks:clonecommand computed its content directory path by substring-splitting__dirnameon/src/or/lib/:In a real installed environment this is unreliable, because the install path itself can contain
/lib/more than once (for example, Node's ownlib/node_modulesdirectory, in addition to this package's own compiledlib/output)..split('/lib/')[0]matches the first occurrence, so it resolved to an unrelated ancestor directory instead of the package's actual root.Every cleanup path in the command — the pre-clone check that clears leftover data, the post-import cleanup on success, and the interrupt-handler cleanup — used this incorrectly-resolved path. All three were faithfully deleting a directory that was essentially always empty, while the real export/import data (resolved correctly and independently in
clone-handler.tsviapath.resolve) was never targeted by any cleanup call.Net effect: content from a completed clone run stayed on disk indefinitely and bled into the next clone of a different stack, causing content-type/entry resolution failures during export.
Fix
Resolve
packageRootby fixed relative directory depth instead of string matching, mirroring the approachclone-handler.tsalready uses:commands/cm/stacksis always exactly 3 directories below the package'ssrc/libroot, so this reliably lands on the package root regardless of how the surrounding path is structured — it doesn't depend on any substring content of the install path at all.Verification
Content directory pathline (cleanup target) and theExport directoryline (actual data path) diverged before the fix, and a subsequent clone into a different stack printed"The export directory is not empty. Existing files in this folder may be overwritten."packageRootresolution against nested (.../node_modules/@scope/cli/node_modules/@scope/pkg/...), flat/hoisted, and pnpm.pnpmvirtual-store install layouts — all resolve correctly.rimrafcalls exist in the package, both scoped to a single fixedcontentssubfolder underpackageRoot, andpathDircannot be overridden by a user-supplied external config (it's set after the config merge, unconditionally).tsc --noEmitandtsc -bboth pass cleanly.Test plan
contents/directory is removed afterward