Skip to content

Commit 8a83696

Browse files
saravmajesticclaude
andcommitted
fix(workspace): address review on the /workspace routes
- Refuse a browser origin on an unsecured server (403), as the Altimate Base registration route does; native clients send no Origin. - Refresh rejects a malformed or non-object JSON body (400) instead of treating it as a session-less refresh that resets every overlay. - `Manage.sync` reports an unhonourable pin as `pin-unresolved`, distinct from `no-binding`; the TUI toast names it. - Tests: await the bind's backfill with a stubbed `fetch` (its detached lookup leaked into `create-then-rebind` in CI), restore the pin env after the file, and cover sync's 500 and both routes' origin refusal. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 20b7824 commit 8a83696

5 files changed

Lines changed: 125 additions & 27 deletions

File tree

‎packages/opencode/src/altimate/workspace/manage.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export interface SyncReport {
8282
* and only one of them is the workspace's memory toggle; a toast that said
8383
* "memory is off" for a failed local read sent the user to a setting that was
8484
* fine. */
85-
gatedBecause?: "flag-off" | "no-binding" | "memory-off" | "read-failed"
85+
gatedBecause?: "flag-off" | "no-binding" | "pin-unresolved" | "memory-off" | "read-failed"
8686
sent: number
8787
failed: number
8888
/** Already present in the workspace at their current payload. */
@@ -230,14 +230,12 @@ export async function sync(directory: string): Promise<SyncReport> {
230230
// altimate_change — the IDE extension's pin outranks the project's own link, as it does for
231231
// the per-write mirror (`memory-sync.resolveBinding`). Without it an extension-launched `serve`
232232
// answered "not linked" for the workspace it was pinned to. Only the pin arm is layered, so an
233-
// unpinned session keeps the cache-only read, and a pin that cannot be honoured stays gated
233+
// unpinned session keeps the cache-only read, and a pin that cannot be honoured stays gated —
234+
// under its own reason, since "nothing is linked" would misdescribe a workspace that exists —
234235
// rather than falling through to the project's link.
235236
const pinned = await resolvePinnedBindingForRouting(directory).catch(() => ({ status: "unknown" as const }))
236-
const binding = pinned
237-
? pinned.status === "bound"
238-
? pinned.binding
239-
: null
240-
: await readLocalBinding(directory).catch(() => null)
237+
if (pinned && pinned.status !== "bound") return gated("pin-unresolved")
238+
const binding = pinned ? pinned.binding : await readLocalBinding(directory).catch(() => null)
241239
if (!binding) return gated("no-binding")
242240

243241
const blocks = await MemoryStore.listAll({ directory }).catch((err) => {

‎packages/opencode/src/plugin/tui/altimate/workspace.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1789,6 +1789,8 @@ function syncMessage(result: Manage.SyncReport): string {
17891789
return "Could not read this project's local memory, so nothing was synced."
17901790
case "no-binding":
17911791
return "Nothing to sync — this project is not linked to a workspace."
1792+
case "pin-unresolved":
1793+
return "Nothing to sync — the pinned workspace could not be confirmed for this project."
17921794
case "flag-off":
17931795
return "Nothing to sync — workspace memory is not enabled in this build."
17941796
default:

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

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,32 @@ globalThis.AI_SDK_LOG_WARNINGS = false
7373

7474
export namespace Server {
7575
const log = Log.create({ service: "server" })
76+
// altimate_change start — shared gate for the `/altimate/workspace/*` routes
77+
/** Why a `/workspace` action must not run, or undefined when it may.
78+
*
79+
* Outside the workspace pilot a skill sync purges the snapshot, so the flag is checked first.
80+
* A browser origin on an unsecured server is refused for the same reason as Altimate Base
81+
* 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. */
83+
function workspaceRouteRefusal(
84+
origin: string | undefined,
85+
): { status: 403 | 409; body: { ok: false; error: string } } | undefined {
86+
if (!CoreFlag.ALTIMATE_WORKSPACE) {
87+
return { status: 409, body: { ok: false, error: "Workspace mode is not enabled for this server." } }
88+
}
89+
if (origin && !Flag.OPENCODE_SERVER_PASSWORD) {
90+
log.warn("refused browser-originated workspace action on an unsecured server", { origin })
91+
return {
92+
status: 403,
93+
body: {
94+
ok: false,
95+
error: "Workspace actions cannot be run from a browser origin on an unsecured server. Set OPENCODE_SERVER_PASSWORD.",
96+
},
97+
}
98+
}
99+
return undefined
100+
}
101+
// altimate_change end
76102
// altimate_change start — the Base credential every provider cache in this process is known to
77103
// reflect: set after the register route has disposed both registries for it. Unset until then,
78104
// because a cache built before a background registration finished cannot be told apart from one
@@ -955,14 +981,26 @@ export namespace Server {
955981
// The `/workspace` menu's Refresh and Sync for the IDE extension, which runs this CLI
956982
// headless and cannot reach the TUI slash command. Both act on the request's instance
957983
// directory and return the `Manage` report as is; wording is the caller's job.
958-
// Refused outside the workspace pilot: with the flag off, a skill sync purges the snapshot.
959984
.post("/altimate/workspace/refresh", async (c) => {
960-
if (!CoreFlag.ALTIMATE_WORKSPACE) {
961-
return c.json({ ok: false, error: "Workspace mode is not enabled for this server." }, 409)
985+
const refused = workspaceRouteRefusal(c.req.header("origin"))
986+
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()
990+
let body: unknown = {}
991+
if (text.trim()) {
992+
try {
993+
body = JSON.parse(text)
994+
} catch {
995+
return c.json({ ok: false, error: "Request body is not valid JSON." }, 400)
996+
}
962997
}
998+
if (body === null || typeof body !== "object" || Array.isArray(body)) {
999+
return c.json({ ok: false, error: "Request body must be a JSON object." }, 400)
1000+
}
1001+
const raw = (body as Record<string, unknown>).sessionID
1002+
const sessionID = typeof raw === "string" && raw ? raw : undefined
9631003
try {
964-
const body = await c.req.json().catch(() => ({}))
965-
const sessionID = typeof body?.sessionID === "string" && body.sessionID ? body.sessionID : undefined
9661004
const Manage = await import("../altimate/workspace/manage")
9671005
// A changed skill snapshot reaches the registry at the start of the next turn
9681006
// (`refreshSkillRegistry` in session/prompt.ts), so nothing is invalidated here.
@@ -975,9 +1013,8 @@ export namespace Server {
9751013
}
9761014
})
9771015
.post("/altimate/workspace/sync", async (c) => {
978-
if (!CoreFlag.ALTIMATE_WORKSPACE) {
979-
return c.json({ ok: false, error: "Workspace mode is not enabled for this server." }, 409)
980-
}
1016+
const refused = workspaceRouteRefusal(c.req.header("origin"))
1017+
if (refused) return c.json(refused.body, refused.status)
9811018
try {
9821019
const Manage = await import("../altimate/workspace/manage")
9831020
const report = await Manage.sync(Instance.directory)

‎packages/opencode/test/altimate/workspace/manage-pin.test.ts‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ const PIN_VARS = [
3737
"ALTIMATE_PINNED_WORKSPACE_NAME",
3838
"ALTIMATE_PINNED_WORKSPACE_ROOT",
3939
]
40+
// Restored in `afterAll`: `bun test` shares one process, and later files must see the pin they had.
41+
const ORIGINAL_PIN = Object.fromEntries(PIN_VARS.map((k) => [k, process.env[k]]))
42+
const originalFetch = globalThis.fetch
4043

4144
/** The pinned workspace has memory ON and the project's own link has it OFF, so the gate reason
4245
* says which of the two the sweep ran against. */
@@ -57,13 +60,19 @@ function clearPin() {
5760
}
5861

5962
async function seedLocalLink(directory = ROOT) {
60-
await recordApprovedBinding(directory, {
61-
datamateId: 7,
62-
datamateName: "project-link",
63-
linkedAt: Date.now(),
64-
repoRemote: "git@example.com:acme/project.git",
65-
projectPath: null,
66-
} as never)
63+
// Awaited, so the bind's skill sync and memory backfill finish inside this test's stubbed
64+
// `fetch` instead of straddling `afterEach` into another file's request log.
65+
await recordApprovedBinding(
66+
directory,
67+
{
68+
datamateId: 7,
69+
datamateName: "project-link",
70+
linkedAt: Date.now(),
71+
repoRemote: "git@example.com:acme/project.git",
72+
projectPath: null,
73+
} as never,
74+
{ awaitBackfill: true },
75+
)
6776
}
6877

6978
beforeEach(() => {
@@ -75,10 +84,13 @@ beforeEach(() => {
7584
({ altimateInstanceName: "acme", altimateUrl: "https://api.test", altimateApiKey: "k" }) as Creds
7685
;(WorkspaceApi as unknown as { listDatamates: () => Promise<unknown> }).listDatamates = async () => WORKSPACES
7786
clearPin()
87+
globalThis.fetch = (async (_input: unknown, _init?: unknown) =>
88+
new Response(JSON.stringify({}), { status: 200, headers: { "content-type": "application/json" } })) as typeof fetch
7889
})
7990

8091
afterEach(() => {
8192
clearPin()
93+
globalThis.fetch = originalFetch
8294
})
8395

8496
afterAll(() => {
@@ -89,6 +101,10 @@ afterAll(() => {
89101
else process.env.XDG_STATE_HOME = ORIGINAL_XDG_STATE_HOME
90102
if (ORIGINAL_PILOT === undefined) delete process.env.ALTIMATE_WORKSPACE
91103
else process.env.ALTIMATE_WORKSPACE = ORIGINAL_PILOT
104+
for (const [k, v] of Object.entries(ORIGINAL_PIN)) {
105+
if (v === undefined) delete process.env[k]
106+
else process.env[k] = v
107+
}
92108
rmSync(SANDBOX, { recursive: true, force: true })
93109
})
94110

@@ -112,14 +128,15 @@ describe("sync under an IDE pin", () => {
112128
setPin("99")
113129
const report = await sync(ROOT)
114130
expect(report.gated).toBe(true)
115-
expect(report.gatedBecause).toBe("no-binding")
131+
// Its own reason: the project IS linked (to 7), so "no-binding" would misdescribe it.
132+
expect(report.gatedBecause).toBe("pin-unresolved")
116133
})
117134

118135
test("a directory outside the pinned root is not treated as pinned", async () => {
119136
setPin()
120137
const report = await sync(OUTSIDE)
121138
expect(report.gated).toBe(true)
122-
expect(report.gatedBecause).toBe("no-binding")
139+
expect(report.gatedBecause).toBe("pin-unresolved")
123140
})
124141
})
125142

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

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ import { disposeAllInstances } from "../fixture/fixture"
1212
const ORIGINAL_FLAG = process.env.ALTIMATE_WORKSPACE
1313
let spies: Array<{ mockRestore: () => void }> = []
1414

15-
function post(path: string, body?: unknown) {
15+
function post(path: string, body?: unknown, headers: Record<string, string> = {}) {
1616
return Server.Default().request(path, {
1717
method: "POST",
18-
headers: { "content-type": "application/json" },
19-
body: body === undefined ? undefined : JSON.stringify(body),
18+
headers: { "content-type": "application/json", ...headers },
19+
body: body === undefined ? undefined : typeof body === "string" ? body : JSON.stringify(body),
2020
})
2121
}
2222

@@ -92,6 +92,34 @@ describe("POST /altimate/workspace/refresh", () => {
9292
expect(response.status).toBe(500)
9393
expect(await response.json()).toEqual({ ok: false, error: "boom" })
9494
})
95+
96+
test("rejects malformed JSON rather than resetting every session's memory", async () => {
97+
const refresh = spyOn(Manage, "refresh")
98+
spies.push(refresh)
99+
100+
const response = await post("/altimate/workspace/refresh", "{not json")
101+
expect(response.status).toBe(400)
102+
expect(((await response.json()) as Record<string, unknown>).ok).toBe(false)
103+
expect(refresh).not.toHaveBeenCalled()
104+
})
105+
106+
test("rejects a body that is not an object", async () => {
107+
const refresh = spyOn(Manage, "refresh")
108+
spies.push(refresh)
109+
110+
expect((await post("/altimate/workspace/refresh", "[]")).status).toBe(400)
111+
expect((await post("/altimate/workspace/refresh", "null")).status).toBe(400)
112+
expect(refresh).not.toHaveBeenCalled()
113+
})
114+
115+
test("refuses a browser origin on an unsecured server", async () => {
116+
const refresh = spyOn(Manage, "refresh")
117+
spies.push(refresh)
118+
119+
const response = await post("/altimate/workspace/refresh", {}, { origin: "https://evil.test" })
120+
expect(response.status).toBe(403)
121+
expect(refresh).not.toHaveBeenCalled()
122+
})
95123
})
96124

97125
describe("POST /altimate/workspace/sync", () => {
@@ -130,4 +158,20 @@ describe("POST /altimate/workspace/sync", () => {
130158
expect((await post("/altimate/workspace/sync")).status).toBe(409)
131159
expect(sync).not.toHaveBeenCalled()
132160
})
161+
162+
test("reports a thrown error as a 500 with its message", async () => {
163+
spies.push(spyOn(Manage, "sync").mockRejectedValue(new Error("boom")))
164+
165+
const response = await post("/altimate/workspace/sync")
166+
expect(response.status).toBe(500)
167+
expect(await response.json()).toEqual({ ok: false, error: "boom" })
168+
})
169+
170+
test("refuses a browser origin on an unsecured server", async () => {
171+
const sync = spyOn(Manage, "sync")
172+
spies.push(sync)
173+
174+
expect((await post("/altimate/workspace/sync", undefined, { origin: "https://evil.test" })).status).toBe(403)
175+
expect(sync).not.toHaveBeenCalled()
176+
})
133177
})

0 commit comments

Comments
 (0)