From 40ce90866e8d05d5d539e4fd4ddf738dcd25e401 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 11 Aug 2026 21:47:45 +0000 Subject: [PATCH] fix(read): resolved tar directory symlinks lazily Kept directory symlinks as one alias node and rewrote requested paths through aliases in ArchiveReader instead of cloning every target descendant during indexing. Full archive materialization now fails explicitly on directory aliases rather than expanding them without a bound. Fixes #4774 --- packages/coding-agent/src/utils/zip.ts | 94 +++++++++++++----------- packages/coding-agent/test/tools.test.ts | 41 +++++++---- 2 files changed, 81 insertions(+), 54 deletions(-) diff --git a/packages/coding-agent/src/utils/zip.ts b/packages/coding-agent/src/utils/zip.ts index 3aee65aa6..d8a879b72 100644 --- a/packages/coding-agent/src/utils/zip.ts +++ b/packages/coding-agent/src/utils/zip.ts @@ -866,11 +866,10 @@ function readTarEntries(rawBytes: Uint8Array): ArchiveIndexEntry[] { throw new ToolError("Not a valid tar archive: missing terminating zero block"); } - // Link records carry no data. Resolve them after all headers are indexed so - // hard links and safe relative symlinks may point forward or form chains. - // Directory symlinks materialize the resolved target subtree at the alias - // path, keeping extraction/rewrite behavior useful without writing symlinks - // that could escape the destination. + // Link records carry no data. Resolve file targets after all headers are + // indexed; directory symlinks remain one alias node and are traversed lazily + // by ArchiveReader so N files behind M aliases never inflate the index to + // N×M entries during a root listing. if (pendingLinks.size > 0) { const entriesByPath = new Map(); for (const entry of entries) entriesByPath.set(entry.path, entry); @@ -881,7 +880,7 @@ function readTarEntries(rawBytes: Uint8Array): ArchiveIndexEntry[] { for (const entry of unresolved) { const pending = pendingLinks.get(entry)!; const target = entriesByPath.get(pending.targetPath); - if (target?.storage) { + if (target?.storage && !target.isDirectory) { entry.size = target.size; entry.storage = target.storage; unresolved.delete(entry); @@ -907,27 +906,8 @@ function readTarEntries(rawBytes: Uint8Array): ArchiveIndexEntry[] { throw new ToolError(`Archive hard link '${entry.path}' targets directory '${pending.targetPath}'`); } - let hasUnresolvedDescendant = false; - for (const candidate of unresolved) { - if (candidate.path.startsWith(targetPrefix)) { - hasUnresolvedDescendant = true; - break; - } - } - if (hasUnresolvedDescendant) continue; - entry.isDirectory = true; - const aliasPrefix = `${entry.path}/`; - const sourceCount = entries.length; - for (let index = 0; index < sourceCount; index++) { - const descendant = entries[index]!; - if (!descendant.path.startsWith(targetPrefix)) continue; - const aliasPath = `${aliasPrefix}${descendant.path.slice(targetPrefix.length)}`; - if (entriesByPath.has(aliasPath)) continue; - const alias: ArchiveIndexEntry = { ...descendant, path: aliasPath }; - entries.push(alias); - entriesByPath.set(aliasPath, alias); - } + entry.storage = { type: "tar-link", targetPath: pending.targetPath }; unresolved.delete(entry); resolved++; } @@ -957,9 +937,7 @@ function extractTarMember(storage: TarStorage, size: number, memberPath: string) } function throwUnreadableTarLink(storage: TarLinkStorage, memberPath: string): never { - throw new ToolError( - `Archive symlink '${memberPath}' cannot be materialized because target '${storage.targetPath}' is unavailable`, - ); + throw new ToolError(`Archive symlink '${memberPath}' cannot be materialized from target '${storage.targetPath}'`); } async function readZipEntries(source: ByteSource): Promise { @@ -1001,7 +979,8 @@ export function parseArchivePathCandidates(filePath: string): ArchivePathCandida /** * An indexed, read-only view over a single archive. ZIP archives are indexed * from the central directory and members are inflated on demand; tar archives - * are parsed from one in-memory buffer and members are sliced on demand. + * are parsed from one in-memory buffer, members are sliced on demand, and + * directory symlink aliases are traversed lazily. */ export class ArchiveReader { readonly format: ArchiveFormat; @@ -1015,6 +994,30 @@ export class ArchiveReader { ensureParentDirectories(this.#entries); } + #resolveDirectoryAliases(archivePath: string): string { + let resolvedPath = archivePath; + const seen = new Set(); + while (true) { + if (seen.has(resolvedPath)) { + throw new ToolError(`Archive path '${archivePath}' crosses a cyclic symlink`); + } + seen.add(resolvedPath); + + const parts = resolvedPath.split("/"); + let replacement: string | undefined; + for (let end = parts.length; end > 0; end--) { + const prefix = parts.slice(0, end).join("/"); + const entry = this.#entries.get(prefix); + if (!entry?.isDirectory || entry.storage?.type !== "tar-link") continue; + const suffix = parts.slice(end).join("/"); + replacement = suffix ? `${entry.storage.targetPath}/${suffix}` : entry.storage.targetPath; + break; + } + if (replacement === undefined) return resolvedPath; + resolvedPath = replacement; + } + } + getNode(subPath?: string): ArchiveNode | undefined { const normalizedPath = normalizeArchiveLookupPath(subPath); if (normalizedPath === undefined) return undefined; @@ -1022,10 +1025,11 @@ export class ArchiveReader { return { path: "", isDirectory: true, size: 0 }; } - const entry = this.#entries.get(normalizedPath); + const resolvedPath = this.#resolveDirectoryAliases(normalizedPath); + const entry = this.#entries.get(resolvedPath); if (!entry) return undefined; return { - path: entry.path, + path: normalizedPath, isDirectory: entry.isDirectory, size: entry.size, mtimeMs: entry.mtimeMs, @@ -1038,8 +1042,9 @@ export class ArchiveReader { throw new ToolError("Archive path cannot contain '..'"); } + const resolvedPath = normalizedPath ? this.#resolveDirectoryAliases(normalizedPath) : ""; if (normalizedPath) { - const entry = this.#entries.get(normalizedPath); + const entry = this.#entries.get(resolvedPath); if (!entry) { throw new ToolError(`Archive path '${normalizedPath}' not found`); } @@ -1048,22 +1053,23 @@ export class ArchiveReader { } } - const prefix = normalizedPath ? `${normalizedPath}/` : ""; + const sourcePrefix = resolvedPath ? `${resolvedPath}/` : ""; const children = new Map(); for (const entry of this.#entries.values()) { - if (normalizedPath) { - if (!entry.path.startsWith(prefix) || entry.path === normalizedPath) continue; + if (resolvedPath) { + if (!entry.path.startsWith(sourcePrefix) || entry.path === resolvedPath) continue; } - const relativePath = normalizedPath ? entry.path.slice(prefix.length) : entry.path; + const relativePath = resolvedPath ? entry.path.slice(sourcePrefix.length) : entry.path; const nextSegment = relativePath.split("/")[0]; if (!nextSegment) continue; const childPath = normalizedPath ? `${normalizedPath}/${nextSegment}` : nextSegment; if (children.has(childPath)) continue; - const childEntry = this.#entries.get(childPath); + const sourceChildPath = resolvedPath ? `${resolvedPath}/${nextSegment}` : nextSegment; + const childEntry = this.#entries.get(sourceChildPath); const isDirectory = childEntry?.isDirectory ?? relativePath.includes("/"); children.set(childPath, { name: nextSegment, @@ -1085,7 +1091,8 @@ export class ArchiveReader { throw new ToolError("Archive file path is required"); } - const entry = this.#entries.get(normalizedPath); + const resolvedPath = this.#resolveDirectoryAliases(normalizedPath); + const entry = this.#entries.get(resolvedPath); if (!entry) { throw new ToolError(`Archive file '${normalizedPath}' not found`); } @@ -1110,7 +1117,7 @@ export class ArchiveReader { bytes = await readZipFileBytes(entry.storage, entry.size); } return { - path: entry.path, + path: normalizedPath, isDirectory: false, size: entry.size, mtimeMs: entry.mtimeMs, @@ -1201,7 +1208,12 @@ export async function readArchiveEntries(source: ArchiveSource): Promise { expect(new TextDecoder().decode(linkedContent)).toBe("shared content\n"); }); - it("should preserve safe relative tar symlink members", async () => { - const archivePath = path.join(testDir, "symlink.tar"); + it("should preserve safe relative tar file symlinks", async () => { + const archivePath = path.join(testDir, "file-symlink.tar"); fs.writeFileSync( archivePath, createTarArchive([ { path: "pkg/lib/tool.js", content: "export const linked = true;\n" }, { path: "pkg/bin/tool", content: "", typeFlag: "2", linkName: "../lib/tool.js" }, - { path: "pkg/current", content: "", typeFlag: "2", linkName: "lib" }, ]), ); @@ -752,22 +751,38 @@ describe("Coding Agent Tools", () => { }); expect(getTextOutput(linkedResult)).toContain("export const linked = true"); - const directoryLinkedResult = await readTool.execute("test-call-tar-directory-symlink-member", { - path: `${archivePath}:pkg/current/tool.js`, - }); - expect(getTextOutput(directoryLinkedResult)).toContain("export const linked = true"); - const entries = await readArchiveEntries(archivePath); const linkedContent = entries.get("pkg/bin/tool"); if (!(linkedContent instanceof Uint8Array)) { throw new Error("Expected symlink content to materialize as bytes"); } expect(new TextDecoder().decode(linkedContent)).toBe("export const linked = true;\n"); - const directoryLinkedContent = entries.get("pkg/current/tool.js"); - if (!(directoryLinkedContent instanceof Uint8Array)) { - throw new Error("Expected directory-symlink content to materialize as bytes"); - } - expect(new TextDecoder().decode(directoryLinkedContent)).toBe("export const linked = true;\n"); + }); + + it("should resolve directory symlinks lazily without materializing subtrees", async () => { + const archivePath = path.join(testDir, "directory-symlinks.tar"); + fs.writeFileSync( + archivePath, + createTarArchive([ + { path: "pkg/lib/tool.js", content: "export const linked = true;\n" }, + { path: "pkg/lib/extra.js", content: "export const extra = true;\n" }, + { path: "pkg/current-a", content: "", typeFlag: "2", linkName: "lib" }, + { path: "pkg/current-b", content: "", typeFlag: "2", linkName: "lib" }, + { path: "pkg/current-c", content: "", typeFlag: "2", linkName: "lib" }, + ]), + ); + + const linkedResult = await readTool.execute("test-call-tar-directory-symlink-member", { + path: `${archivePath}:pkg/current-a/tool.js`, + }); + expect(getTextOutput(linkedResult)).toContain("export const linked = true"); + + const directoryResult = await readTool.execute("test-call-tar-directory-symlink-directory", { + path: `${archivePath}:pkg/current-b`, + }); + expect(getTextOutput(directoryResult)).toContain("extra.js"); + + await expect(readArchiveEntries(archivePath)).rejects.toThrow(/cannot be materialized/); }); it("should list dangling tar symlinks but reject their materialization", async () => {