Skip to content

chore(sync): preview upstream/dev (2026-02-06)#8

Closed
aryasaatvik wants to merge 13 commits intodevfrom
codex/merge-upstream-dev-20260206-3
Closed

chore(sync): preview upstream/dev (2026-02-06)#8
aryasaatvik wants to merge 13 commits intodevfrom
codex/merge-upstream-dev-20260206-3

Conversation

@aryasaatvik
Copy link
Member

@aryasaatvik aryasaatvik commented Feb 6, 2026

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):

git push --force-with-lease origin codex/rebase-upstream-dev-20260206-3:dev

Open with Devin

aryasaatvik and others added 12 commits February 5, 2026 15:28
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
@github-actions
Copy link

github-actions bot commented Feb 6, 2026

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 12 additional findings in Devin Review.

Open in Devin Review

Comment on lines 78 to 92
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)
}),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 247 to 252
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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines 80 to 86
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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines 247 to 251
return
}
match.abort.abort()
for (const item of match.callbacks) {
item.reject(new DOMException("Aborted", "AbortError"))
}
delete s[sessionID]
SessionStatus.set(sessionID, { type: "idle" })

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@aryasaatvik
Copy link
Member Author

Applied to dev via force-push from codex/rebase-upstream-dev-20260206-3 (origin/dev now at 2b326cd). Closing preview PR.

@aryasaatvik aryasaatvik closed this Feb 6, 2026
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 13 additional findings in Devin Review.

Open in Devin Review

Comment on lines +152 to +153
outfile: `dist/${name}/bin/opencoder`,
execArgv: [`--user-agent=opencoder/${Script.version}`, "--use-system-ca", "--"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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:21 which defines "opencode": "./bin/opencode"
  • packages/opencode/Dockerfile:10 and Dockerfile:13 which copy from bin/opencode
  • packages/desktop/scripts/prepare.ts:19 and predev.ts:9 which reference bin/opencode
  • nix/desktop.nix:75, nix/opencode.nix:53-56 which install from bin/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.

Suggested change
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", "--"],
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant