chore(sync): preview upstream/dev (2026-02-06)#8
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
# Conflicts: # AGENTS.md # bun.lock # packages/app/package.json # packages/app/src/components/session/session-header.tsx # packages/app/src/components/terminal.tsx # packages/app/src/context/file.tsx # packages/app/src/context/language.tsx # packages/app/src/pages/session.tsx # packages/console/app/package.json # packages/console/app/src/routes/workspace/[id]/graph-section.tsx # packages/console/core/package.json # packages/console/function/package.json # packages/console/mail/package.json # packages/enterprise/package.json # packages/function/package.json # packages/opencode/package.json # packages/opencode/src/acp/agent.ts # packages/opencode/src/config/config.ts # packages/opencode/src/plugin/copilot.ts # packages/opencode/src/plugin/index.ts # packages/plugin/package.json # packages/sdk/js/package.json # packages/slack/package.json # packages/ui/package.json # packages/ui/src/components/session-turn.tsx # packages/util/package.json
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
| await Promise.all( | ||
| list.map(async (skill) => { | ||
| const root = path.join(cache, skill.name) | ||
| await Promise.all( | ||
| skill.files.map(async (file) => { | ||
| const link = new URL(file, `${host}/${skill.name}/`).href | ||
| const dest = path.join(root, file) | ||
| await mkdir(path.dirname(dest), { recursive: true }) | ||
| await get(link, dest) | ||
| }), | ||
| ) | ||
|
|
||
| const md = path.join(root, "SKILL.md") | ||
| if (await Bun.file(md).exists()) result.push(root) | ||
| }), |
There was a problem hiding this comment.
🔴 Remote skill download can write files outside cache via path traversal in index.json
Remote skill indexes can provide skill.name or files[] values containing ../ (or absolute paths). Those values are directly passed into path.join(cache, skill.name) and path.join(root, file).
Actual behavior: a malicious index can cause OpenCode to create directories and write arbitrary files outside Global.Path.cache/skills (e.g. skill.name="../../..", file=".ssh/authorized_keys").
Expected behavior: downloaded content should be confined to the skills cache directory.
Root Cause & Impact
In Discovery.pull, path components from the network are treated as trusted paths:
const root = path.join(cache, skill.name)const dest = path.join(root, file)await mkdir(path.dirname(dest), { recursive: true })await Bun.write(dest, ...)
Because path.join will resolve .. segments and allow absolute paths, a crafted index can escape the intended cache directory.
Impact: arbitrary file write on the machine running the server (within the permissions of the OpenCode process). This is a serious security issue.
Prompt for agents
In packages/opencode/src/skill/discovery.ts, harden Discovery.pull() to prevent path traversal when writing downloaded skill files.
1) Before using skill.name, validate it is a safe single path segment (no path separators, no '..', not empty). If invalid, skip the skill with a warning.
2) For each entry in skill.files, validate it is a safe relative path (must not be absolute, must not contain '..' segments after path.posix.normalize or path.normalize). If invalid, skip that file.
3) After computing dest = path.join(root, file), ensure dest is still under root (e.g. by checking that path.resolve(dest).startsWith(path.resolve(root) + path.sep)). If not, skip.
Apply these checks before mkdir/Bun.write. Add logs for skipped entries.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return | ||
| } | ||
| match.abort.abort() | ||
| for (const item of match.callbacks) { | ||
| item.reject(new DOMException("Aborted", "AbortError")) | ||
| } | ||
| delete s[sessionID] | ||
| SessionStatus.set(sessionID, { type: "idle" }) | ||
| return |
There was a problem hiding this comment.
🔴 SessionPrompt.loop waiters can hang forever after cancel because queued callbacks are never rejected
When SessionPrompt.loop(sessionID) is called while the session is already busy, it returns a Promise and queues {resolve, reject} callbacks in state()[sessionID].callbacks.
Actual behavior: cancel(sessionID) aborts the AbortController and deletes the state entry but no longer rejects queued callback promises. Any callers awaiting loop() during a busy session can hang indefinitely.
Expected behavior: canceling a session should promptly reject all queued loop() waiters.
Root Cause & Impact
The code path that queues waiters is:
if (!abort) return new Promise((resolve, reject) => { state()[sessionID].callbacks.push({ resolve, reject }) })(packages/opencode/src/session/prompt.ts:255-262)
But cancel() now only aborts and deletes state:
match.abort.abort()delete s[sessionID]
and no longer iterates match.callbacks to reject them (packages/opencode/src/session/prompt.ts:241-252). The Instance.state cleanup hook also aborts without rejecting callbacks (packages/opencode/src/session/prompt.ts:70-74).
Impact: callers waiting for a running session to finish can deadlock (never resolve/reject), leading to stuck UI/API calls and leaked promises.
(Refers to lines 241-252)
Prompt for agents
Fix SessionPrompt cancellation to reject queued loop waiters.
In packages/opencode/src/session/prompt.ts:
1) In cancel(sessionID) (around lines 241-252), after match.abort.abort() and before deleting state, iterate match.callbacks and call reject(new DOMException("Aborted", "AbortError")) for each (or another consistent abort error type used elsewhere). Then clear match.callbacks.
2) In the Instance.state cleanup function (around lines 70-74), after item.abort.abort(), also reject any queued callbacks for that session for the same reason, to avoid hanging waiters during instance teardown.
Keep behavior consistent with previous implementation that rejected callbacks.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d7231fde1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const root = path.join(cache, skill.name) | ||
| await Promise.all( | ||
| skill.files.map(async (file) => { | ||
| const link = new URL(file, `${host}/${skill.name}/`).href | ||
| const dest = path.join(root, file) | ||
| await mkdir(path.dirname(dest), { recursive: true }) | ||
| await get(link, dest) |
There was a problem hiding this comment.
Validate skill file paths before writing to disk
The new remote-skill downloader writes files using path.join(root, file) where file comes directly from the remote index. If a malicious or compromised skills index returns entries like ../.bashrc or ../../some/other/path, path.join will escape root and Bun.write will overwrite arbitrary files that the user can access. This is a path traversal issue reachable whenever skills.urls is enabled and the URL points to untrusted or tampered content. Consider resolving the destination and rejecting any path that doesn’t stay under root (e.g., const dest = path.resolve(root, file); if (!dest.startsWith(root + path.sep)) return;).
Useful? React with 👍 / 👎.
| return | ||
| } | ||
| match.abort.abort() | ||
| for (const item of match.callbacks) { | ||
| item.reject(new DOMException("Aborted", "AbortError")) | ||
| } | ||
| delete s[sessionID] | ||
| SessionStatus.set(sessionID, { type: "idle" }) |
There was a problem hiding this comment.
Reject queued prompt callers on cancel
When a session is already running, loop() returns a Promise that is stored in callbacks (see lines 255–261). After this change, cancel() aborts and deletes the session state without rejecting those queued callbacks, so any concurrent callers waiting on loop() will remain pending forever if the session is canceled. This can hang API requests or UI actions that are awaiting the prompt completion. Restore rejecting those callbacks on cancel to ensure callers are notified of the abort.
Useful? React with 👍 / 👎.
|
Applied to dev via force-push from codex/rebase-upstream-dev-20260206-3 (origin/dev now at 2b326cd). Closing preview PR. |
| outfile: `dist/${name}/bin/opencoder`, | ||
| execArgv: [`--user-agent=opencoder/${Script.version}`, "--use-system-ca", "--"], |
There was a problem hiding this comment.
🔴 Binary output filename changed to 'opencoder' but package.json and infrastructure still reference 'opencode'
The build script was updated to output the compiled binary as opencoder instead of opencode, but the package.json bin field and all infrastructure files (Dockerfiles, Nix configs, desktop sidecar references) still reference the old opencode filename.
Root Cause
The build script at packages/opencode/script/build.ts:152 changed the output filename from dist/${name}/bin/opencode to dist/${name}/bin/opencoder, but this change was not propagated to:
packages/opencode/package.json:21which defines"opencode": "./bin/opencode"packages/opencode/Dockerfile:10andDockerfile:13which copy frombin/opencodepackages/desktop/scripts/prepare.ts:19andpredev.ts:9which referencebin/opencodenix/desktop.nix:75,nix/opencode.nix:53-56which install frombin/opencode- Multiple other infrastructure files
Impact: The compiled binary will be named opencoder but when users run npm install -g opencode-ai, the package.json bin field will try to symlink to a non-existent ./bin/opencode file, causing the installation to fail or the CLI to be unusable. Docker builds will fail, Nix packages will fail, and the desktop app won't find its sidecar binary.
| outfile: `dist/${name}/bin/opencoder`, | |
| execArgv: [`--user-agent=opencoder/${Script.version}`, "--use-system-ca", "--"], | |
| outfile: `dist/${name}/bin/opencode`, | |
| execArgv: [`--user-agent=opencode/${Script.version}`, "--use-system-ca", "--"], |
Was this helpful? React with 👍 or 👎 to provide feedback.
Review-friendly preview of syncing upstream/dev into dev.
Note: we are treating GitHub Actions as non-blocking for this workflow (local checks are the source of truth).
Apply branch (linear rebase): codex/rebase-upstream-dev-20260206-3
Upstream: 9f00b8c
Origin dev (before): fa9655b
Apply after approval (local workflow):