smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(read): bounded tar directory-alias resolution and resolved aliased link targets

Browse Source 
- Capped directory-alias rewrites per lookup (ELOOP-style, 40) so a directory
  symlink targeting its own subtree (a -> a/b) throws a catchable ToolError
  instead of looping forever growing the path.
- Deferred pending tar link resolution while any directory on the target path
  is itself an unresolved link, and rewrote targets through established
  directory aliases before the exact-path lookup, so file symlinks routed
  through directory aliases materialize instead of dangling.
- Regression tests reproduce both shapes: pre-fix the aliased symlink read
  failed with 'cannot be materialized' and the self-cycle read hung.
This commit is contained in:
can1357
2026-08-12 02:02:33 +02:00
parent 5ffafec146
commit 7e2663360f
3 changed files with 102 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+1
View File
@@ -21,6 +21,7 @@
- Fixed the MCP Streamable HTTP transport never sending the `MCP-Protocol-Version` header and negotiating the stale `2025-03-26` revision, which made spec-current servers (e.g. AWS Bedrock AgentCore Gateway with an outbound per-user OAuth target) reject every `tools/call` with a generic internal error. The client now negotiates `2025-11-25`, echoes the negotiated version on every request after `initialize`, and resumes server-closed POST response streams with `Last-Event-ID` after the requested SSE retry interval ([#8264](https://github.com/can1357/oh-my-pi/issues/8264)).
- Fixed `/handoff` losing the previous session's `local://` artifacts (plans, scratch files, research notes): the handoff document referenced files that became unreadable because the new session's `local/` root was empty. Local artifacts are now copied across the handoff session boundary, mirroring the plan approve-and-execute path ([#8261](https://github.com/can1357/oh-my-pi/issues/8261)).
- Fixed valid `.tar` and `.tar.gz` archive reads terminating omp through libarchive by parsing tar members in-process ([#4774](https://github.com/can1357/oh-my-pi/issues/4774)).
- Fixed the in-process tar reader looping forever on directory symlinks targeting their own subtree (`a -> a/b`) and misclassifying file symlinks routed through directory aliases as dangling; alias resolution is now depth-bounded and link targets resolve through directory aliases at index time.
## [17.2.14] - 2026-08-11
packages/coding-agent/src/utils/zip.ts
+65 -35
View File
@@ -908,11 +908,36 @@ function readTarEntries(rawBytes: Uint8Array): ArchiveIndexEntry[] {
for (const entry of entries) entriesByPath.set(entry.path, entry);
const unresolved = new Set(pendingLinks.keys());
// True while the target itself or any directory on its path is a link
// that has not been classified yet; such targets must wait a pass so a
// file symlink routed through a directory alias is not misjudged
// dangling before the alias resolves.
const dependsOnUnresolvedLink = (targetPath: string): boolean => {
const parts = targetPath.split("/");
for (let end = parts.length; end > 0; end--) {
const prefixEntry = entriesByPath.get(parts.slice(0, end).join("/"));
if (prefixEntry && unresolved.has(prefixEntry)) return true;
}
return false;
};
while (unresolved.size > 0) {
let resolved = 0;
for (const entry of unresolved) {
const pending = pendingLinks.get(entry)!;
const target = entriesByPath.get(pending.targetPath);
if (dependsOnUnresolvedLink(pending.targetPath)) continue;
// Targets may route through directory aliases classified in an
// earlier pass; rewrite before the exact-path lookup.
let targetPath = pending.targetPath;
try {
targetPath = resolveDirectoryAliasPath(entriesByPath, targetPath);
} catch {
// Cyclic alias chain: fall through to the dangling-symlink path.
}
if (targetPath !== pending.targetPath && dependsOnUnresolvedLink(targetPath)) continue;
const target = entriesByPath.get(targetPath);
if (target?.storage && !target.isDirectory) {
entry.size = target.size;
entry.storage = target.storage;
@@ -920,12 +945,11 @@ function readTarEntries(rawBytes: Uint8Array): ArchiveIndexEntry[] {
resolved++;
continue;
}
if (target && unresolved.has(target)) continue;
// An empty target is the archive root, which is always a directory.
const targetPrefix = `${pending.targetPath}/`;
const targetPrefix = `${targetPath}/`;
const targetIsDirectory =
pending.targetPath === "" ||
targetPath === "" ||
target?.isDirectory === true ||
entries.some(candidate => candidate.path.startsWith(targetPrefix));
if (!targetIsDirectory) {
@@ -976,6 +1000,40 @@ function throwUnreadableTarLink(storage: TarLinkStorage, memberPath: string): ne
throw new ToolError(`Archive symlink '${memberPath}' cannot be materialized from target '${storage.targetPath}'`);
}
/** ELOOP-style bound on directory-alias rewrites during a single path lookup. */
const MAX_LINK_RESOLUTION_DEPTH = 40;
/**
* Rewrite `archivePath` through directory symlink aliases until it no longer
* crosses one. Bounded: an exact revisit and an alias chain that keeps growing
* the path (e.g. a directory symlink targeting its own subtree, `a -> a/b`)
* both throw a catchable cyclic-symlink error instead of looping forever.
*/
function resolveDirectoryAliasPath(entries: ReadonlyMap<string, ArchiveIndexEntry>, archivePath: string): string {
let resolvedPath = archivePath;
const seen = new Set<string>();
for (let depth = 0; depth < MAX_LINK_RESOLUTION_DEPTH && !seen.has(resolvedPath); depth++) {
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 = entries.get(prefix);
if (!entry?.isDirectory || entry.storage?.type !== "tar-link") continue;
const suffix = parts.slice(end).join("/");
replacement = suffix
? entry.storage.targetPath
? `${entry.storage.targetPath}/${suffix}`
: suffix
: entry.storage.targetPath;
break;
}
if (replacement === undefined) return resolvedPath;
resolvedPath = replacement;
}
throw new ToolError(`Archive path '${archivePath}' crosses a cyclic symlink`);
}
async function readZipEntries(source: ByteSource): Promise<ArchiveIndexEntry[]> {
const directoryInfo = await readZipCentralDirectoryInfo(source);
const centralDirectory = await source.read(directoryInfo.offset, directoryInfo.offset + directoryInfo.size);
@@ -1030,34 +1088,6 @@ export class ArchiveReader {
ensureParentDirectories(this.#entries);
}
#resolveDirectoryAliases(archivePath: string): string {
let resolvedPath = archivePath;
const seen = new Set<string>();
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
? `${entry.storage.targetPath}/${suffix}`
: 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;
@@ -1065,7 +1095,7 @@ export class ArchiveReader {
return { path: "", isDirectory: true, size: 0 };
}
const resolvedPath = this.#resolveDirectoryAliases(normalizedPath);
const resolvedPath = resolveDirectoryAliasPath(this.#entries, normalizedPath);
if (resolvedPath === "") {
return { path: normalizedPath, isDirectory: true, size: 0 };
}
@@ -1085,7 +1115,7 @@ export class ArchiveReader {
throw new ToolError("Archive path cannot contain '..'");
}
const resolvedPath = normalizedPath ? this.#resolveDirectoryAliases(normalizedPath) : "";
const resolvedPath = normalizedPath ? resolveDirectoryAliasPath(this.#entries, normalizedPath) : "";
if (normalizedPath && resolvedPath !== "") {
const entry = this.#entries.get(resolvedPath);
if (!entry) {
@@ -1134,7 +1164,7 @@ export class ArchiveReader {
throw new ToolError("Archive file path is required");
}
const resolvedPath = this.#resolveDirectoryAliases(normalizedPath);
const resolvedPath = resolveDirectoryAliasPath(this.#entries, normalizedPath);
if (resolvedPath === "") {
throw new ToolError(`Archive path '${normalizedPath}' is a directory`);
}
packages/coding-agent/test/tools.test.ts
+36
View File
@@ -921,6 +921,42 @@ describe("Coding Agent Tools", () => {
await expect(readArchiveEntries(archivePath)).rejects.toThrow(/cannot be materialized/);
});
it("should resolve file symlinks routed through directory symlinks", async () => {
const archivePath = path.join(testDir, "aliased-file-symlink.tar");
fs.writeFileSync(
archivePath,
createTarArchive([
// The file symlink precedes the directory alias it routes
// through, so resolution must defer until the alias settles.
{ path: "pkg/bin/tool", content: "", typeFlag: "2", linkName: "../current/tool.js" },
{ path: "pkg/lib/tool.js", content: "export const linked = true;\n" },
{ path: "pkg/current", content: "", typeFlag: "2", linkName: "lib" },
]),
);
const linkedResult = await readTool.execute("test-call-tar-aliased-symlink-member", {
path: `${archivePath}:pkg/bin/tool`,
});
expect(getTextOutput(linkedResult)).toContain("export const linked = true");
});
it("should reject directory symlinks targeting their own subtree instead of looping", async () => {
const archivePath = path.join(testDir, "self-cycle-symlink.tar");
fs.writeFileSync(
archivePath,
createTarArchive([
{ path: "a/b/f.txt", content: "unreachable\n" },
// `a -> a/b` grows the resolved path on every rewrite; the
// pre-fix resolver looped forever on this shape.
{ path: "a", content: "", typeFlag: "2", linkName: "a/b" },
]),
);
await expect(
readTool.execute("test-call-tar-self-cycle", { path: `${archivePath}:a/b/f.txt` }),
).rejects.toThrow(/cyclic or unsupported links/);
});
it("should resolve tar symlinks whose target is the archive root", async () => {
const archivePath = path.join(testDir, "root-symlinks.tar");
fs.writeFileSync(