fix(cursor): resolve symlinks when confining resource downloads
A lexical containment check is not containment. `out/config` is relative and `..`-free, so it passed - while a `ws/out -> /elsewhere` link inside the workspace sent the write straight out. Proven before the fix: the download landed in the link's target. Containment now realpaths three things: the target when it exists, the immediate link destination when it is a dangling symlink (a write still follows it), and otherwise the deepest existing ancestor with the not-yet-created segments re-applied. Each branch has a regression, and all three fail the suite when individually reverted. Also moves this branch's ai/catalog changelog entries back under [Unreleased]; two commits had re-landed them inside the released [17.1.5] section, which left `packages/ai/CHANGELOG.md` with two. All three released sections are now byte-identical to upstream/main. (cherry picked from commit e3ed4035ab8a1781c49a68c1fbe92c88bec3aa25)
This commit is contained in:
Diogo Soares Rodrigues
committed by
can1357
can1357
parent
7a944f1baa
commit
91b703b6af
@@ -53,21 +53,6 @@
|
||||
|
||||
- Added `getProxyForUrl()` for transports that need provider-specific and standard proxy environment resolution with `NO_PROXY` support ([#6770](https://github.com/can1357/oh-my-pi/issues/6770)).
|
||||
- Added SiliconFlow and SiliconFlow (China) to the built-in API-key login provider catalog so `omp login siliconflow` / `omp login siliconflow-cn` stores a reusable credential validated against each region's `/v1/models` endpoint.
|
||||
### Changed
|
||||
|
||||
- The Cursor Pi arg translation (`piReadPath`, `piJoinPath`, `piLsPath`, `piEscapeRegexLiteral`, `piLimit`) moved to `providers/cursor-pi-args`, re-exported from `providers/cursor/exec-modern` so existing imports are unaffected. The legacy pi shim shares these helpers and is compiled into the bundled virtual module registry, where a nested `providers/<dir>/<mod>` specifier is unresolvable under bunfs — and importing them from the exec module would drag the whole protobuf graph in for two string functions.
|
||||
|
||||
## [17.1.5] - 2026-07-27
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed OpenAI Responses replay treating a tool output as paired with a matching call that appeared later in the input, or a tool call as paired with an earlier output. Pair repair now respects wire order before preserving or synthesizing each side.
|
||||
- Fixed adaptive-thinking Anthropic models omitting the interleaved-thinking beta on signature-enforcing proxies, which caused persisted interleaved assistant turns to fail on replay ([#6717](https://github.com/can1357/oh-my-pi/issues/6717)).
|
||||
- Kimi Code now sends its session-stable prompt cache key on both supported transports: `prompt_cache_key` for OpenAI-compatible requests and `metadata.user_id` for Anthropic-compatible requests. Explicit keys survive side-channel session IDs, while `cacheRetention: "none"` still disables automatic affinity ([#6049](https://github.com/can1357/oh-my-pi/issues/6049)).
|
||||
- Fresh encrypted auth-broker snapshot caches are revalidated within a short startup budget, so one-shot clients see newly imported or revoked credentials immediately when the broker is reachable while retaining cache fallback for transport and server failures.
|
||||
- Fixed custom `anthropic-messages` endpoints dropping native web-search call/result blocks in the leaked-thinking wrapper, preserving signed continuation history in source order without carrying a preceding text signature onto later unsigned blocks ([#6703](https://github.com/can1357/oh-my-pi/issues/6703)).
|
||||
### Added
|
||||
|
||||
- Cursor's modern exec wire protocol is now handled end to end. `agent.proto` models the frames current Cursor CLI builds emit — the seven Pi tools (`ExecServerMessage` 45-51), hooks, subagents, allowlist prechecks, MCP state, smart-mode classification, canvas diagnostics, conversation search, agent-store conflicts and git diff — and every one of them gets a typed answer. The Pi frames run their local equivalents (`read`/`bash`/`edit`/`write`/`grep`/`glob`); the rest answer with the error, not-found or empty-but-valid variant that is actually true of this client. Frames this build cannot name at all now raise `ExecClientControlMessage.throw` with `unknown_exec_variant`, and recognised frames with no truthful answer (`git_diff_request`, whose `GetDiffResponse` has no error variant) raise `exec_variant_unsupported`, instead of a silent ack that leaves the server waiting.
|
||||
- `lsp` is advertised in the MCP tool catalog again. It was filtered out as a Cursor-native tool, but the native `diagnostics` frame covers one of roughly ten LSP actions, so the other nine were unreachable.
|
||||
|
||||
|
||||
@@ -32,6 +32,7 @@
|
||||
### Added
|
||||
|
||||
- Added SiliconFlow providers (`siliconflow`, `siliconflow-cn`) with dynamic-only OpenAI-compatible model discovery: no bundled catalog — the model list is fetched live from each region's `/v1/models` endpoint, with non-chat entries (embedding, reranker, image, audio, video) filtered out. Discovery hydrates pricing, context/output limits, and reasoning metadata from the provider's models.dev catalog at runtime (with bundled upstream references as a reasoning-only fallback for ids models.dev has not indexed), so reasoning models keep thinking enabled and sessions compact against real context windows. `SILICONFLOW_API_KEY` / `SILICONFLOW_CN_API_KEY` environment variables are wired into `getEnvApiKey`.
|
||||
- Regenerated the Cursor agent protobufs (`discovery/cursor-gen/agent_pb.ts`) against the modern `agent.proto`, adding the message and enum families current Cursor CLI builds emit: Pi tool exec frames, hook queries and responses, subagents, allowlist prechecks, MCP state, smart-mode classification, canvas diagnostics, conversation search, agent-store conflicts and git diff. Purely additive — no existing exported symbol changed shape.
|
||||
|
||||
## [17.1.5] - 2026-07-27
|
||||
|
||||
@@ -39,9 +40,6 @@
|
||||
|
||||
- Fixed Kimi Code (`kimi-code`) reporting `maxTokens: 32000` for every model — its `/coding/v1/models` discovery mapper and the bundled catalog applied a blanket constant, truncating `k3`/`k3-256k` output at ~4x below their real 131072 ceiling and `kimi-for-coding`/`kimi-for-coding-highspeed` below their 32768 ceiling. Output caps are now derived per family, and the model cache is invalidated so upgrades drop the stale `maxTokens: 32000` rows (including the discovery-only `k3-256k`) instead of serving them until the next network refresh ([#6711](https://github.com/can1357/oh-my-pi/issues/6711)).
|
||||
- Fixed Anthropic model discovery 404ing when the registry derived the provider base URL from a bundled model without the `/v1` suffix (`https://api.anthropic.com/models` instead of `/v1/models`), which let a stale text-only cache row shadow fresh models.dev vision metadata — surfacing as snapcompact refusing to run on `claude-opus-5`. Discovery now always targets `/v1/models` while model rows keep the provider base URL ([#6563](https://github.com/can1357/oh-my-pi/issues/6563)).
|
||||
### Added
|
||||
|
||||
- Regenerated the Cursor agent protobufs (`discovery/cursor-gen/agent_pb.ts`) against the modern `agent.proto`, adding the message and enum families current Cursor CLI builds emit: Pi tool exec frames, hook queries and responses, subagents, allowlist prechecks, MCP state, smart-mode classification, canvas diagnostics, conversation search, agent-store conflicts and git diff. Purely additive — no existing exported symbol changed shape.
|
||||
|
||||
## [17.1.4] - 2026-07-26
|
||||
|
||||
|
||||
@@ -143,7 +143,7 @@
|
||||
- Fixed `pi_bash` killing commands that explicitly asked for no deadline. `timeout` is `optional int32` and `bash` documents `0` as "disables the command deadline", but a truthiness check folded a supplied `0` into unset, applying the 300s default instead. A present `0` now passes through; negatives, which have no local meaning and would otherwise clamp to the 1s floor, still fall back to the default.
|
||||
- Fixed the Cursor exec bridge granting `edit` and `grep` to sessions that withheld them. Both bridge-only tools are constructed rather than looked up, and `executeTool` prefers a constructed override over the registry, so a restricted tool set (`toolNames` without them, or `restrictToolNames`) still got a working `pi_edit`/`pi_grep` — native frames arrive regardless of the advertised catalog. Both are now gated on the session having actually granted the tool, matching the `delete` frame's existing check (issue #5680).
|
||||
- Fixed Cursor advisor bridge tools bypassing approval settings. The advisor's `pi_edit`/`pi_grep` instances are approval-wrapped, but the wrapper reads `tools.approvalMode`, per-tool `tools.approval.<tool>` policies and `autoApprove` only from the execute-time tool context — which the advisor bridge never supplied, so every native advisor frame resolved as `yolo` with empty policies and ran past a configured `ask` or `deny`. Advisors now receive the same context store as the primary bridge.
|
||||
- Fixed Cursor's `list_mcp_resources`/`read_mcp_resource` frames answering as though the client hosted no MCP servers. The bridge hardcoded an empty catalog and `not_found`, so resources from servers the session held live connections to were invisible to the model even while the same session read them through `mcp://`. Both frames now answer from the session's `MCPManager`; a lookup failure surfaces as an error rather than an empty catalog, which would read as "asked, none exist". A read carrying `download_path` writes the resource to that path and answers with the path alone, per the wire contract, instead of putting the payload back in the model's context — confined to the workspace, since that path arrives from the server and the general-purpose resolver deliberately honors absolute paths and `..`.
|
||||
- Fixed Cursor's `list_mcp_resources`/`read_mcp_resource` frames answering as though the client hosted no MCP servers. The bridge hardcoded an empty catalog and `not_found`, so resources from servers the session held live connections to were invisible to the model even while the same session read them through `mcp://`. Both frames now answer from the session's `MCPManager`; a lookup failure surfaces as an error rather than an empty catalog, which would read as "asked, none exist". A read carrying `download_path` writes the resource to that path and answers with the path alone, per the wire contract, instead of putting the payload back in the model's context — confined to the workspace, since that path arrives from the server and the general-purpose resolver deliberately honors absolute paths and `..`. Containment resolves symlinks (target, dangling link target, and deepest existing ancestor), because a relative `..`-free path through a link that points outward still writes outward.
|
||||
- Fixed the Cursor native `delete` frame bypassing approval settings. Unlike every other frame it removes the file directly instead of running a registry tool, so no approval wrapper sat in front of it — `allowNativeDelete` answers whether a mutating tool was granted, which is a different question from whether the user's policy allows the call. A configured `tools.approval.delete: deny`, or an `always-ask` session that this channel cannot prompt in, now refuses the frame and keeps the file.
|
||||
- Fixed `pi_ls` never reporting that a listing was clipped. The bridge read the entry cap from a flat `details.resultLimitReached`, which `glob` sets but `read` — the tool serving `pi_ls` — does not: it records the cap through `OutputMeta` at `details.meta.limits.resultLimit.reached`. Every capped listing therefore reached Cursor with `entry_limit_reached` unset, reading as complete. Both shapes are now checked, the same way the truncation translation already handles its two producers.
|
||||
|
||||
|
||||
@@ -526,9 +526,16 @@ export function resolveToCwd(filePath: string, cwd: string): string {
|
||||
* {@link resolveToCwd} deliberately honors absolute paths, `~`, and `..` —
|
||||
* correct for a path a user typed, wrong for one a remote peer supplied.
|
||||
* Callers handling untrusted input (Cursor's `download_path`) use this instead:
|
||||
* only a non-empty relative path resolving under the live cwd is accepted, so
|
||||
* only a non-empty relative path landing under the live cwd is accepted, so
|
||||
* neither `/etc/passwd` nor `../../escape` can be written through.
|
||||
*
|
||||
* The lexical check alone is not containment: a symlink inside the workspace
|
||||
* can point anywhere, so `out/config` under a `ws/out -> /elsewhere` link is
|
||||
* relative, `..`-free, and still writes outside. Both the target and its
|
||||
* deepest existing ancestor are therefore realpath-resolved — the ancestor
|
||||
* because a download names a file that does not exist yet, so the link in its
|
||||
* path is the only thing that can be resolved before the write.
|
||||
*
|
||||
* The cwd itself is rejected: a download names a file, never the directory.
|
||||
*/
|
||||
export function confineToWorkspace(filePath: string, cwd: string): string | null {
|
||||
@@ -538,9 +545,69 @@ export function confineToWorkspace(filePath: string, cwd: string): string | null
|
||||
if (filePath.startsWith("~") || isInternalUrlPath(filePath)) return null;
|
||||
const root = path.resolve(cwd);
|
||||
const resolved = path.resolve(root, filePath);
|
||||
const relative = path.relative(root, resolved);
|
||||
if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) return null;
|
||||
return resolved;
|
||||
if (!isUnderRootLexical(resolved, root)) return null;
|
||||
|
||||
// A workspace reached through a link of its own is legitimate (/tmp on
|
||||
// macOS), so the real root is the comparison basis. An unresolvable root is
|
||||
// not a workspace to contain anything in.
|
||||
const realRoot = tryRealpath(root);
|
||||
if (!realRoot) return null;
|
||||
|
||||
// An existing target is authoritative: resolve it outright.
|
||||
const realTarget = tryRealpath(resolved);
|
||||
if (realTarget) return isUnderRootLexical(realTarget, realRoot) ? resolved : null;
|
||||
|
||||
// `realpath` also fails on a *dangling* link, and a write follows that link
|
||||
// to wherever it points. Resolving one level answers where the bytes would
|
||||
// actually land; anything unresolvable stays refused.
|
||||
const linkTarget = tryReadlink(resolved);
|
||||
if (linkTarget !== null) {
|
||||
const dest = path.resolve(path.dirname(resolved), linkTarget);
|
||||
const realDest = tryRealpath(path.dirname(dest));
|
||||
if (!realDest) return null;
|
||||
return isUnderRootLexical(path.join(realDest, path.basename(dest)), realRoot) ? resolved : null;
|
||||
}
|
||||
|
||||
// Otherwise walk up to the deepest ancestor that does exist and check that,
|
||||
// then re-apply the segments below it. Those segments are `..`-free by the
|
||||
// lexical check above, so they cannot climb back out.
|
||||
let ancestor = path.dirname(resolved);
|
||||
const tail: string[] = [path.basename(resolved)];
|
||||
for (;;) {
|
||||
const real = tryRealpath(ancestor);
|
||||
if (real) {
|
||||
return isUnderRootLexical(path.join(real, ...tail.reverse()), realRoot) ? resolved : null;
|
||||
}
|
||||
const parent = path.dirname(ancestor);
|
||||
// Ran past the root without finding anything real: the workspace itself
|
||||
// resolved above, so this cannot happen unless it vanished mid-check.
|
||||
if (parent === ancestor || !isUnderRootLexical(ancestor, root)) return null;
|
||||
tail.push(path.basename(ancestor));
|
||||
ancestor = parent;
|
||||
}
|
||||
}
|
||||
|
||||
/** Whether `target` is a strict descendant of `root`, ignoring symlinks. */
|
||||
function isUnderRootLexical(target: string, root: string): boolean {
|
||||
const relative = path.relative(root, target);
|
||||
return !!relative && !relative.startsWith("..") && !path.isAbsolute(relative);
|
||||
}
|
||||
|
||||
function tryRealpath(target: string): string | null {
|
||||
try {
|
||||
return fs.realpathSync.native(target);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/** The immediate link target, or `null` when the path is not a symlink. */
|
||||
function tryReadlink(target: string): string | null {
|
||||
try {
|
||||
return fs.readlinkSync(target);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export function formatPathRelativeToCwd(
|
||||
|
||||
@@ -762,6 +762,49 @@ describe("CursorExecHandlers mounted tool bridge", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses a download path that escapes through a symlink", async () => {
|
||||
// A lexical check is not containment. `out/config` is relative and
|
||||
// `..`-free, but with `ws/out` linked outside the workspace the write
|
||||
// lands wherever the link points — as does a write to a dangling link.
|
||||
const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "cursor-mcp-symlink-"));
|
||||
try {
|
||||
const inner = path.join(workspace, "ws");
|
||||
const outside = path.join(workspace, "outside");
|
||||
await fs.mkdir(inner);
|
||||
await fs.mkdir(outside);
|
||||
await fs.symlink(outside, path.join(inner, "out"));
|
||||
await fs.symlink(path.join(outside, "dangling.txt"), path.join(inner, "link.txt"));
|
||||
// A link whose target already exists: the write would overwrite the
|
||||
// real file out there rather than create anything new.
|
||||
await Bun.write(path.join(outside, "existing.txt"), "original");
|
||||
await fs.symlink(path.join(outside, "existing.txt"), path.join(inner, "existing.txt"));
|
||||
const handlers = new CursorExecHandlers({
|
||||
cwd: inner,
|
||||
tools: new Map(),
|
||||
mcpResources: {
|
||||
serverNames: () => ["files"],
|
||||
getServerResources: () => undefined,
|
||||
readServerResource: async (_name, uri) => ({ contents: [{ uri, text: "payload" }] }),
|
||||
},
|
||||
});
|
||||
|
||||
for (const escape of ["out/config", "out/deep/nested.txt", "link.txt", "existing.txt"]) {
|
||||
await expect(
|
||||
handlers.readMcpResource({ server: "files", uri: "files://x", downloadPath: escape }),
|
||||
).rejects.toThrow(/outside the workspace/);
|
||||
}
|
||||
expect(await Array.fromAsync(new Bun.Glob("**/*").scan({ cwd: outside }))).toEqual(["existing.txt"]);
|
||||
expect(await Bun.file(path.join(outside, "existing.txt")).text()).toBe("original");
|
||||
|
||||
// A real workspace path still downloads: the guard resolves links, it
|
||||
// does not refuse every path whose parents do not exist yet.
|
||||
await handlers.readMcpResource({ server: "files", uri: "files://x", downloadPath: "deep/new/file.txt" });
|
||||
expect(await Bun.file(path.join(inner, "deep/new/file.txt")).text()).toBe("payload");
|
||||
} finally {
|
||||
await removeWithRetries(workspace);
|
||||
}
|
||||
});
|
||||
|
||||
it("answers nothing when the session has no MCP manager", async () => {
|
||||
// A host without MCP must still answer truthfully rather than throwing:
|
||||
// an empty catalog and `not_found` are the honest responses.
|
||||
|
||||
Reference in New Issue
Block a user