fix(tarball): match npm path stripping (#14247)
Pacquet normalized away a leading dot component before dropping the archive's top-level path segment. As a result, `./package/package.json` became `package.json`, while pnpm and npm's `tar.x({ strip: 1 })` count `.` as the stripped component and retain `package/package.json`.
Preserve a leading dot in shared archive segmentation long enough for the tar extractor to remove exactly one component. Absolute paths and parent traversal remain rejected. Add matching Rust and TypeScript regression coverage.
Closes pnpm/pnpm#14165
This commit is contained in:
1 parent
a22206394e
commit
6f787c19c6
4 files changed
+36
-6
No files matched your search
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"pacquet": patch
|
||||
---
|
||||
|
||||
Pacquet now strips exactly one leading path component from `./`-prefixed tarball entries, matching pnpm and npm's tar extraction semantics and keeping shared store keys consistent.
|
||||
@@ -959,6 +959,9 @@ pub(crate) fn tar_entry_payload<'a, Reader: std::io::Read>(
|
||||
/// both implementations share to a reader that *does* treat them as
|
||||
/// separators.
|
||||
///
|
||||
/// A leading `.` is preserved because npm's `tar` counts it as the
|
||||
/// component removed by `strip: 1`. Other `.` components are ignored.
|
||||
///
|
||||
/// `None` for an absolute path or one climbing past the root.
|
||||
pub(crate) fn archive_entry_segments(raw: &str) -> Option<Vec<String>> {
|
||||
let normalized = raw.replace('\\', "/");
|
||||
@@ -966,9 +969,11 @@ pub(crate) fn archive_entry_segments(raw: &str) -> Option<Vec<String>> {
|
||||
return None;
|
||||
}
|
||||
let mut segments = Vec::new();
|
||||
for segment in normalized.split('/') {
|
||||
for (index, segment) in normalized.split('/').enumerate() {
|
||||
match segment {
|
||||
"" | "." => {}
|
||||
"" => {}
|
||||
"." if index == 0 => segments.push(segment.to_string()),
|
||||
"." => {}
|
||||
".." => return None,
|
||||
other => segments.push(other.to_string()),
|
||||
}
|
||||
|
||||
@@ -5312,6 +5312,21 @@ fn tar_with_raw_entry_name(name: &[u8]) -> Vec<u8> {
|
||||
tar_bytes
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_strips_only_one_component_from_a_dot_prefixed_entry_path() {
|
||||
let (tempdir, store_path) = tempdir_with_leaked_path();
|
||||
|
||||
let tar_bytes = tar_with_raw_entry_name(b"./package/package.json");
|
||||
let (cas_paths, pkg_files_idx) =
|
||||
extract_tarball_entries(&tar_bytes, store_path, None).expect("extract the tarball");
|
||||
|
||||
assert_eq!(cas_paths.keys().collect::<Vec<_>>(), vec!["package/package.json"]);
|
||||
assert_eq!(pkg_files_idx.files.keys().collect::<Vec<_>>(), vec!["package/package.json"]);
|
||||
assert!(pkg_files_idx.manifest.is_none());
|
||||
|
||||
drop(tempdir);
|
||||
}
|
||||
|
||||
/// A backslash is an ordinary filename character on Unix but a
|
||||
/// separator on Windows, and these keys travel between the two through
|
||||
/// the shared `index.db`. pnpm folds `\` to `/` before validating
|
||||
|
||||
@@ -251,7 +251,7 @@ test('unpack a tarball that contains hard links', () => {
|
||||
// A malicious tarball entry like "foo\..\..\..\.npmrc" should have its path normalized
|
||||
test('path traversal with backslashes is blocked (Windows security fix)', () => {
|
||||
// Create a minimal valid tarball with a malicious filename
|
||||
const tarBuffer = createTarballWithEntry('foo\\..\\..\\..\\malicious.txt', 'evil content')
|
||||
const tarBuffer = createTarballWithEntry('package/foo\\..\\..\\..\\malicious.txt', 'evil content')
|
||||
|
||||
const result = parseTarball(tarBuffer)
|
||||
const fileNames = Array.from(result.files.keys())
|
||||
@@ -263,16 +263,21 @@ test('path traversal with backslashes is blocked (Windows security fix)', () =>
|
||||
}
|
||||
})
|
||||
|
||||
test('only one segment is stripped from a dot-prefixed tarball entry', () => {
|
||||
const result = parseTarball(createTarballWithEntry('./package/package.json', '{}'))
|
||||
|
||||
expect(Array.from(result.files.keys())).toStrictEqual(['package/package.json'])
|
||||
})
|
||||
|
||||
// Helper to create a minimal tarball buffer with a single entry
|
||||
function createTarballWithEntry (fileName: string, content: string): Buffer {
|
||||
function createTarballWithEntry (entryPath: string, content: string): Buffer {
|
||||
const contentBytes = Buffer.from(content, 'utf8')
|
||||
|
||||
// Create a 512-byte header
|
||||
const header = Buffer.alloc(512, 0)
|
||||
|
||||
// File name at offset 0 (max 100 chars)
|
||||
const nameToWrite = `package/${fileName}`
|
||||
header.write(nameToWrite, 0, Math.min(nameToWrite.length, 100), 'utf8')
|
||||
header.write(entryPath, 0, Math.min(entryPath.length, 100), 'utf8')
|
||||
|
||||
// File mode at offset 100 (octal, 8 bytes) - 0644
|
||||
header.write('0000644\0', 100, 8, 'utf8')
|
||||
|
||||
Reference in new issue
Block a user