Skip to content

Commit 8af7760

Browse files
saravmajesticclaude
saravmajestic
and
claude
committed
fix(workspace): tighten /workspace refresh input and origin checks
- A present `sessionID` must be a non-empty string (400), must exist (404), and must belong to the request's directory (409), since the reload loads this directory's workspace memory into that session. - With a server password set, refuse cross-origin requests too: a browser replays cached Basic credentials on a cross-site form POST. Same-origin pages and Origin-less native clients are unaffected. - Read the request body inside a guard, so a failed read keeps the route's `{ ok: false, error }` 500 contract. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
1 parent 8a83696 commit 8af7760

2 files changed

Lines changed: 90 additions & 14 deletions

File tree

‎packages/opencode/src/server/server.ts‎

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@ import { FreeTierConsent } from "../altimate/free/consent"
4545
import { InstanceStore } from "@/project/instance-store"
4646
import { AppRuntime } from "@/effect/app-runtime"
4747
// altimate_change end
48-
// altimate_change - `/workspace` Refresh and Sync are gated on the workspace pilot flag
48+
// altimate_change - `/workspace` Refresh and Sync: pilot flag gate, and the session-directory check
4949
import { Flag as CoreFlag } from "@opencode-ai/core/flag/flag"
50+
import nodePath from "node:path"
51+
import { Session } from "../session"
5052
// altimate_change end
5153
import { FileRoutes } from "./routes/file"
5254
import { ConfigRoutes } from "./routes/config"
@@ -79,14 +81,16 @@ export namespace Server {
7981
* Outside the workspace pilot a skill sync purges the snapshot, so the flag is checked first.
8082
* A browser origin on an unsecured server is refused for the same reason as Altimate Base
8183
* registration: a CORS-allowed page is not a local process. Native clients (the extension host,
82-
* curl) send no Origin; with a server password set, basicAuth has already vetted the caller. */
84+
* curl) send no Origin. With a server password set, a same-origin page may call; others may not. */
8385
function workspaceRouteRefusal(
8486
origin: string | undefined,
87+
host: string | undefined,
8588
): { status: 403 | 409; body: { ok: false; error: string } } | undefined {
8689
if (!CoreFlag.ALTIMATE_WORKSPACE) {
8790
return { status: 409, body: { ok: false, error: "Workspace mode is not enabled for this server." } }
8891
}
89-
if (origin && !Flag.OPENCODE_SERVER_PASSWORD) {
92+
if (!origin) return undefined
93+
if (!Flag.OPENCODE_SERVER_PASSWORD) {
9094
log.warn("refused browser-originated workspace action on an unsecured server", { origin })
9195
return {
9296
status: 403,
@@ -96,8 +100,21 @@ export namespace Server {
96100
},
97101
}
98102
}
103+
// With a password set, basicAuth has vetted the credentials — but a browser replays cached
104+
// Basic credentials on a cross-site form POST too, so only this server's own pages may call.
105+
if (!sameOrigin(origin, host)) {
106+
log.warn("refused cross-origin workspace action", { origin })
107+
return { status: 403, body: { ok: false, error: "Workspace actions cannot be run from another origin." } }
108+
}
99109
return undefined
100110
}
111+
export function sameOrigin(origin: string, host: string | undefined): boolean {
112+
try {
113+
return !!host && new URL(origin).host === host
114+
} catch {
115+
return false
116+
}
117+
}
101118
// altimate_change end
102119
// altimate_change start — the Base credential every provider cache in this process is known to
103120
// reflect: set after the register route has disposed both registries for it. Unset until then,
@@ -982,11 +999,19 @@ export namespace Server {
982999
// headless and cannot reach the TUI slash command. Both act on the request's instance
9831000
// directory and return the `Manage` report as is; wording is the caller's job.
9841001
.post("/altimate/workspace/refresh", async (c) => {
985-
const refused = workspaceRouteRefusal(c.req.header("origin"))
1002+
const refused = workspaceRouteRefusal(c.req.header("origin"), c.req.header("host"))
9861003
if (refused) return c.json(refused.body, refused.status)
987-
// An absent or empty body is a session-less refresh; a malformed one is an error, not a
988-
// silent fall-back to resetting every session's memory overlay.
989-
const text = await c.req.text()
1004+
// An absent or empty body is a session-less refresh; anything else must be well formed.
1005+
// Falling back to "no session" on bad input would silently widen the operation to
1006+
// resetting every session's memory overlay.
1007+
let text: string
1008+
try {
1009+
text = await c.req.text()
1010+
} catch (err) {
1011+
const error = err instanceof Error ? err.message : String(err)
1012+
log.error("workspace refresh: could not read the request body", { error })
1013+
return c.json({ ok: false, error }, 500)
1014+
}
9901015
let body: unknown = {}
9911016
if (text.trim()) {
9921017
try {
@@ -999,7 +1024,24 @@ export namespace Server {
9991024
return c.json({ ok: false, error: "Request body must be a JSON object." }, 400)
10001025
}
10011026
const raw = (body as Record<string, unknown>).sessionID
1002-
const sessionID = typeof raw === "string" && raw ? raw : undefined
1027+
if (raw !== undefined && (typeof raw !== "string" || !raw)) {
1028+
return c.json({ ok: false, error: "sessionID must be a non-empty string." }, 400)
1029+
}
1030+
const sessionID = raw as string | undefined
1031+
// The memory reload loads THIS directory's workspace memory into the named session, so the
1032+
// session must be one of this directory's; another project's would receive it.
1033+
if (sessionID) {
1034+
const session = await Session.get(sessionID as never).catch((err) => err as Error)
1035+
if (session instanceof NotFoundError) {
1036+
return c.json({ ok: false, error: `Session not found: ${sessionID}` }, 404)
1037+
}
1038+
if (session instanceof Error) {
1039+
return c.json({ ok: false, error: `Invalid sessionID: ${sessionID}` }, 400)
1040+
}
1041+
if (nodePath.resolve(session.directory) !== nodePath.resolve(Instance.directory)) {
1042+
return c.json({ ok: false, error: "That session belongs to a different project directory." }, 409)
1043+
}
1044+
}
10031045
try {
10041046
const Manage = await import("../altimate/workspace/manage")
10051047
// A changed skill snapshot reaches the registry at the start of the next turn
@@ -1013,7 +1055,7 @@ export namespace Server {
10131055
}
10141056
})
10151057
.post("/altimate/workspace/sync", async (c) => {
1016-
const refused = workspaceRouteRefusal(c.req.header("origin"))
1058+
const refused = workspaceRouteRefusal(c.req.header("origin"), c.req.header("host"))
10171059
if (refused) return c.json(refused.body, refused.status)
10181060
try {
10191061
const Manage = await import("../altimate/workspace/manage")

‎packages/opencode/test/server/altimate-workspace-routes.test.ts‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"
77
import { Server } from "../../src/server/server"
88
import * as Manage from "../../src/altimate/workspace/manage"
9+
import { Session } from "../../src/session"
10+
import { NotFoundError } from "../../src/storage/db"
911
import { resetDatabase } from "./db"
1012
import { disposeAllInstances } from "../fixture/fixture"
1113

@@ -35,9 +37,10 @@ afterEach(async () => {
3537

3638
describe("POST /altimate/workspace/refresh", () => {
3739
test("returns the refresh report and passes the session through", async () => {
40+
spies.push(spyOn(Session, "get").mockResolvedValue({ directory: process.cwd() } as never))
3841
const refresh = spyOn(Manage, "refresh").mockResolvedValue({
3942
skillsChanged: true,
40-
memory: { ok: true, status: "reloaded", blocks: 4 } as unknown as Manage.RefreshReport["memory"],
43+
memory: { ok: true, status: "loaded", count: 4 },
4144
errors: [],
4245
})
4346
spies.push(refresh)
@@ -52,6 +55,25 @@ describe("POST /altimate/workspace/refresh", () => {
5255
expect(refresh.mock.calls[0][1]).toBe("ses_123")
5356
})
5457

58+
test("refuses a session that belongs to another project directory", async () => {
59+
spies.push(spyOn(Session, "get").mockResolvedValue({ directory: "/somewhere/else" } as never))
60+
const refresh = spyOn(Manage, "refresh")
61+
spies.push(refresh)
62+
63+
const response = await post("/altimate/workspace/refresh", { sessionID: "ses_123" })
64+
expect(response.status).toBe(409)
65+
expect(refresh).not.toHaveBeenCalled()
66+
})
67+
68+
test("answers 404 for a session that does not exist", async () => {
69+
spies.push(spyOn(Session, "get").mockRejectedValue(new NotFoundError({ message: "Session not found" })))
70+
const refresh = spyOn(Manage, "refresh")
71+
spies.push(refresh)
72+
73+
expect((await post("/altimate/workspace/refresh", { sessionID: "ses_123" })).status).toBe(404)
74+
expect(refresh).not.toHaveBeenCalled()
75+
})
76+
5577
test("works without a body, leaving the memory overlay to reload on the next turn", async () => {
5678
const refresh = spyOn(Manage, "refresh").mockResolvedValue({
5779
skillsChanged: false,
@@ -66,12 +88,14 @@ describe("POST /altimate/workspace/refresh", () => {
6688
expect(refresh.mock.calls[0][1]).toBeUndefined()
6789
})
6890

69-
test("ignores a session id that is not a string", async () => {
70-
const refresh = spyOn(Manage, "refresh").mockResolvedValue({ skillsChanged: false, errors: [] })
91+
test("rejects a session id that is present but not a non-empty string", async () => {
92+
const refresh = spyOn(Manage, "refresh")
7193
spies.push(refresh)
7294

73-
await post("/altimate/workspace/refresh", { sessionID: 42 })
74-
expect(refresh.mock.calls[0][1]).toBeUndefined()
95+
// Falling back to "no session" would widen the refresh to every session's memory overlay.
96+
expect((await post("/altimate/workspace/refresh", { sessionID: 42 })).status).toBe(400)
97+
expect((await post("/altimate/workspace/refresh", { sessionID: "" })).status).toBe(400)
98+
expect(refresh).not.toHaveBeenCalled()
7599
})
76100

77101
test("is refused outside the workspace pilot, without touching the snapshot", async () => {
@@ -175,3 +199,13 @@ describe("POST /altimate/workspace/sync", () => {
175199
expect(sync).not.toHaveBeenCalled()
176200
})
177201
})
202+
203+
describe("same-origin check used when a server password is set", () => {
204+
test("accepts this server's own pages only", () => {
205+
expect(Server.sameOrigin("http://127.0.0.1:4096", "127.0.0.1:4096")).toBe(true)
206+
expect(Server.sameOrigin("https://evil.test", "127.0.0.1:4096")).toBe(false)
207+
expect(Server.sameOrigin("http://127.0.0.1:9999", "127.0.0.1:4096")).toBe(false)
208+
expect(Server.sameOrigin("null", "127.0.0.1:4096")).toBe(false)
209+
expect(Server.sameOrigin("http://127.0.0.1:4096", undefined)).toBe(false)
210+
})
211+
})

0 commit comments

Comments
 (0)