fix(server-mode): close the gaps the review of workspace sharing found

- A Terminal tab a collaborator opens records them (dashboard_runs.started_by):
  its pane token acts as them — and only while still shared with them —
  never as the workspace owner (userFromPaneToken).
- POST /api/run always starts the lane's AI tool: a collaborator gets
  ECOLLABSTART whatever the body says (shell: true no longer slips past).
- A collaborator disabled since no longer locks the share list: kept on it
  (shown disabled) or taken off; only a new addition must be enabled.
- The workspace home tells a collaborator to open an existing session and
  keeps Start disabled instead of failing on every submit.
This commit is contained in:
2026-11-06 10:48:29 +07:00
parent 0bf552b793
commit 54001585e2
16 changed files with 201 additions and 31 deletions
+12 -2
View File
@@ -58,6 +58,7 @@ import { ProxyPicker, proxyArg, useAiProxies, useSessionProxy } from "../run/Pro
import { Select } from "../Select";
import { WorkspaceRowMenu, type WorkspaceMenuAction } from "./WorkspaceRowMenu";
import { WorkspaceShareModal } from "./WorkspaceShareModal";
import { usePermissions } from "../AuthGate";
import { workspaceName } from "./WorkspaceRow";
import { SessionMenu } from "./SessionMenu";
import { menuPoint, useContextMenu } from "../ContextMenu";
@@ -119,6 +120,7 @@ function HomeBody({
const { t } = useTranslation("lanes");
const navigate = useNavigate();
const aiTools = useAiTools();
const { ownsLane } = usePermissions();
// The NEW session's AI tool — its own choice, the workspace's by default.
const [tool, setTool] = useState(lane.tool);
const entry = aiTools.byId(tool);
@@ -272,10 +274,13 @@ function HomeBody({
};
const text = promptWithImages(prompt, images, claude);
// A workspace shared with this user: they join its open sessions, but start
// none of their own yet (the server answers ECOLLABSTART).
const collab = !ownsLane(lane);
const start = async () => {
// Enter reaches here past the disabled button: the same guards again.
// A ref, not only the state: two Enters in one tick both read `starting` false.
if (startingRef.current || uploading || !text) return;
if (collab || startingRef.current || uploading || !text) return;
startingRef.current = true;
setStarting(true);
setError(null);
@@ -531,7 +536,7 @@ function HomeBody({
type="button"
data-testid="workspace-home-start"
onClick={() => void start()}
disabled={starting || uploading || !text}
disabled={collab || starting || uploading || !text}
aria-label={t("home.start")}
title={t("home.start")}
className="flex h-8 w-8 items-center justify-center rounded-full bg-accent-solid text-white transition-opacity hover:opacity-90 disabled:opacity-40"
@@ -541,6 +546,11 @@ function HomeBody({
</div>
</div>
</div>
{collab && (
<p data-testid="workspace-home-collab" className="mt-2 text-xs text-fg-muted">
{t("shared.noStart")}
</p>
)}
{/* The session's quieter choices, under the box like Claude Desktop's. */}
<div
data-testid="workspace-home-options"
@@ -19,7 +19,7 @@ import { confirmDialog } from "../../lib/confirm";
import { useDialog } from "../../hooks/useDialog";
import { Checkbox } from "../Checkbox";
type Person = { id: number; username: string; name: string };
type Person = { id: number; username: string; name: string; disabled?: boolean };
export function WorkspaceShareModal({ lane, onClose }: { lane: Lane; onClose: () => void }) {
const { t } = useTranslation("lanes");
@@ -36,8 +36,14 @@ export function WorkspaceShareModal({ lane, onClose }: { lane: Lane; onClose: ()
Promise.all([api.artifacts.people(), api.lanes.shares(lane.id)])
.then(([p, s]) => {
if (!alive) return;
// Never the owner: a workspace is not shared with them.
setPeople(p.people.filter((x) => x.id !== lane.owner_id));
// Never the owner: a workspace is not shared with them. Someone it is
// already shared with but who was disabled since is not in `people`:
// listed anyway, so they can be taken off.
const known = new Set(p.people.map((x) => x.id));
setPeople([
...p.people.filter((x) => x.id !== lane.owner_id),
...s.shares.filter((x) => !known.has(x.id)).map((x) => ({ ...x, disabled: true })),
]);
const ids = new Set(s.shares.map((x) => x.id));
setInitial(ids);
setPicked(new Set(ids));
@@ -163,6 +169,9 @@ export function WorkspaceShareModal({ lane, onClose }: { lane: Lane; onClose: ()
{p.name !== p.username && (
<span className="ml-1.5 text-fg-muted">@{p.username}</span>
)}
{p.disabled && (
<span className="ml-1.5 text-fg-muted">{t("share.disabled")}</span>
)}
</span>
}
/>
@@ -103,6 +103,25 @@ describe("WorkspaceShareModal", () => {
expect(onClose).toHaveBeenCalled();
});
it("lists a collaborator disabled since, so they can be taken off", async () => {
m.shares.mockResolvedValue({
shares: [
{ id: 2, username: "bob", name: "Bob", sharedAt: "" },
{ id: 9, username: "dan", name: "Dan", sharedAt: "", disabled: true },
],
canManage: true,
});
render(<WorkspaceShareModal lane={lane} onClose={() => {}} />);
await settle();
const dan = screen.getByRole("checkbox", { name: /Dan/ });
expect(dan).toHaveAttribute("aria-checked", "true");
expect(dan).toHaveTextContent("(disabled)");
fireEvent.click(dan);
fireEvent.click(screen.getByTestId("workspace-share-save"));
await settle();
expect(m.setShares).toHaveBeenCalledWith(7, [2]);
});
it("takes someone off without asking", async () => {
render(<WorkspaceShareModal lane={lane} onClose={() => {}} />);
await settle();
+2
View File
@@ -402,7 +402,9 @@
"share.confirm": "Share",
"share.saved_one": "Shared with {{count}} person",
"share.saved_other": "Shared with {{count}} people",
"share.disabled": "(disabled)",
"share.savedNone": "No longer shared with anyone",
"shared.noStart": "This workspace is shared with you: open one of its sessions to work in it. Only its owner starts new sessions for now.",
"shared.badge": "Shared with you",
"panel.changes": "Changes",
"panel.analyst": "Analyst",
+2
View File
@@ -402,7 +402,9 @@
"share.confirm": "Chia sẻ",
"share.saved_one": "Đã chia sẻ với {{count}} người",
"share.saved_other": "Đã chia sẻ với {{count}} người",
"share.disabled": "(đã vô hiệu hoá)",
"share.savedNone": "Không còn chia sẻ với ai",
"shared.noStart": "Workspace này được chia sẻ với bạn: mở một session có sẵn của nó để làm việc. Hiện chỉ chủ workspace mới mở được session mới.",
"shared.badge": "Được chia sẻ với bạn",
"panel.changes": "Thay đổi",
"panel.analyst": "Analyst",
+8 -1
View File
@@ -941,7 +941,14 @@ export type PoolProfile = {
active: boolean;
};
/** A user a workspace is shared with (`/api/lanes/:id/shares`) — names only. */
export type LaneShare = { id: number; username: string; name: string; sharedAt: string };
export type LaneShare = {
id: number;
username: string;
name: string;
sharedAt: string;
/** Disabled since it was shared: kept on the list, granting nothing. */
disabled?: boolean;
};
/** A login profile of a tool other than Claude (`/api/tool-profiles`) — never a token. */
export type ToolProfileIdentity = {
/** codex: `chatgpt` | `apikey`; omp: `credentials`. */
+4 -1
View File
@@ -2111,7 +2111,10 @@ owner or an admin only (`403 ENOTOWNER`), same-origin, each change audited. Loca
mode: `GET` answers an empty list, `PUT` `400 ELOCALMODE`. A user it is shared with
sees and works in it like its owner, but `PATCH {title}`, `reset`/`remove`/`purge`,
`DELETE` and `PUT …/shares` answer them `403 ENOTOWNER`, and a non-shell
`POST /api/lanes/:id/start` or `POST /api/run` `403 ECOLLABSTART`. Users taken off
`POST /api/lanes/:id/start` or any `POST /api/run` `403 ECOLLABSTART`. A Terminal
tab they open (`start {shell: true}`) records them in `dashboard_runs.started_by`:
its pane token acts as them, not the owner. A user disabled since stays on the
list (`disabled: true`) and may be kept or removed; only a new one must be enabled. Users taken off
get the WebSocket message `lane_access` `{laneId, userIds}` (sent to them only). See
`docs/SERVER-MODE.md` → Sharing a workspace.
+10 -2
View File
@@ -257,9 +257,17 @@ decides the account. They can read everything in the owner's home (the owner's
other workspaces, SSH keys, AI logins, the pane tokens of the owner's panes)
and their commits carry the owner's git identity; only AgentHub's `audit_log`
records who did what. The Share dialog says so. Share only with people you
would hand your keyboard to.
would hand your keyboard to. Two consequences follow and are accepted, not
bugs: a collaborator can read the pane token of a pane the OWNER started (and
so act as the owner through the CLI from it), and `ECOLLABSTART` is a
dashboard rule, not a barrier — in a terminal they can run `claude` as the
owner's account. A Terminal tab the collaborator opens records them as its
starter (`dashboard_runs.started_by`), so its own pane token acts as THEM, and
only while the workspace is still shared with them (`userFromPaneToken`).
Every share and unshare is audited (`share workspace <id>` /
A user already on the list who was disabled since stays listed (marked
disabled, granting nothing) and can be taken off; only a NEW addition must be
enabled. Every share and unshare is audited (`share workspace <id>` /
`unshare workspace <id>`). Taking it back sends the users taken off
`lane_access` (their list re-reads) and closes their open terminals on it at
the next attach re-check. Deleting the workspace, or the user, drops the rows;
+6
View File
@@ -1,6 +1,12 @@
# Workspace & session sharing — plan
Status: **phase 1 built 2026-11-06** (workspace sharing; R1 decided as option **(a)** — accept and warn hard). Phases 2–4 not built.
An independent review of phase 1 found and got fixed: a collaborator's Terminal
tab acted as the owner through its pane token (now `dashboard_runs.started_by`),
`POST /api/run {shell:true}` bypassed `ECOLLABSTART`, a disabled collaborator
locked the share list, and the home's prompt box let a collaborator type into a
Start that always failed. Accepted under (a): the owner's own panes' tokens are
readable by a collaborator, and `ECOLLABSTART` is advisory under S3.
Server mode only. In local mode there are no users, so there is nothing to share
and every piece below is a no-op (`isServerMode()`).
+77 -7
View File
@@ -5,8 +5,10 @@
* the list (`GET/PUT /api/lanes/:id/shares`, audited, enabled users only, never
* the owner); the user it is shared with sees the workspace and its sessions
* but may not rename, reset, remove or delete it (`ENOTOWNER`) and starts no AI
* session in it yet (`ECOLLABSTART`); a user it is not shared with still gets
* 404; taking it back ends their access at once.
* session in it yet (`ECOLLABSTART`, `/api/run` whatever the body says); a
* Terminal tab they open acts as THEM through its pane token, never as the
* owner; a collaborator disabled since does not block the list; a user it is
* not shared with still gets 404; taking it back ends their access at once.
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
*/
@@ -83,8 +85,7 @@ after(() => {
}
});
const laneIds = async (as) =>
(await call("GET", "/api/lanes", { as })).body.lanes.map((l) => l.id);
const laneIds = async (as) => (await call("GET", "/api/lanes", { as })).body.lanes.map((l) => l.id);
describe("sharing a workspace", () => {
it("is managed only by its owner (or an admin), for enabled users other than the owner", async () => {
@@ -127,7 +128,9 @@ describe("sharing a workspace", () => {
"ENOTOWNER"
);
assert.equal(
code(await call("POST", `/api/lanes/${laneA}/remove`, { as: "bob", body: { confirm: true } })),
code(
await call("POST", `/api/lanes/${laneA}/remove`, { as: "bob", body: { confirm: true } })
),
"ENOTOWNER"
);
assert.equal(code(await call("DELETE", `/api/lanes/${laneA}`, { as: "bob" })), "ENOTOWNER");
@@ -167,6 +170,72 @@ describe("sharing a workspace", () => {
});
assert.equal(run.status, 403);
assert.equal(run.body.error.code, "ECOLLABSTART");
// /api/run has no shell: `shell: true` does not get an AI session past it.
const shell = await call("POST", "/api/run", {
as: "bob",
body: { laneId: laneA, shell: true, initialPrompt: "hi" },
});
assert.equal(shell.body.error.code, "ECOLLABSTART");
});
it("makes a pane token act as the user who opened that pane, never the owner", () => {
const tmux = require("../lib/tmux");
const real = tmux.hasSession;
tmux.hasSession = () => true;
try {
const tokenFor = (runId, startedBy) => {
const cred = auth.newPaneToken();
db.prepare(
`INSERT OR REPLACE INTO dashboard_runs (id, mode, cwd, status, lane_id, tool, pane_token_hash, started_by)
VALUES (?, 'interactive', '/w/alice', 'running', ?, 'shell', ?, ?)`
).run(runId, laneA, cred.hash, startedBy);
return cred.token;
};
const asUser = (token) =>
auth.userFromPaneToken({
headers: { "x-agenthub-pane-token": token },
socket: { remoteAddress: "127.0.0.1" },
});
assert.equal(asUser(tokenFor(`agenthub-lane-${laneA}-1`, null))?.username, "alice");
const bobs = tokenFor(`agenthub-lane-${laneA}-2`, users.bob.id);
assert.equal(asUser(bobs)?.username, "bob");
// No longer shared with him: his pane's token is nobody.
db.prepare("DELETE FROM lane_shares WHERE lane_id = ? AND user_id = ?").run(
laneA,
users.bob.id
);
assert.equal(asUser(bobs), null);
db.prepare("INSERT INTO lane_shares (lane_id, user_id) VALUES (?, ?)").run(
laneA,
users.bob.id
);
} finally {
tmux.hasSession = real;
db.prepare("DELETE FROM dashboard_runs WHERE lane_id = ?").run(laneA);
}
});
it("is not blocked by a collaborator disabled since", async () => {
const put = (userIds) =>
call("PUT", `/api/lanes/${laneA}/shares`, { as: "alice", body: { userIds } });
assert.equal((await put([users.bob.id, users.carol.id])).status, 200);
db.prepare("UPDATE users SET disabled = 1 WHERE id = ?").run(users.carol.id);
try {
const listed = await call("GET", `/api/lanes/${laneA}/shares`, { as: "alice" });
assert.equal(listed.body.shares.find((u) => u.id === users.carol.id)?.disabled, true);
// Kept on the list while another change is saved, then taken off.
assert.equal((await put([users.bob.id, users.carol.id])).status, 200);
const r = await put([users.bob.id]);
assert.equal(r.status, 200);
assert.deepEqual(
r.body.shares.map((u) => u.username),
["bob"]
);
// But never newly added while disabled.
assert.equal((await put([users.bob.id, users.carol.id])).body.error.code, "EBADUSER");
} finally {
db.prepare("UPDATE users SET disabled = 0 WHERE id = ?").run(users.carol.id);
}
});
it("ends their access as soon as it is taken back, and says so in the audit log", async () => {
@@ -179,8 +248,9 @@ describe("sharing a workspace", () => {
assert.ok(!(await laneIds("bob")).includes(laneA));
assert.equal((await call("GET", "/api/sessions/sA", { as: "bob" })).status, 404);
assert.equal(
db.prepare("SELECT COUNT(*) AS n FROM audit_log WHERE path = ?").get(`unshare workspace ${laneA}`)
.n,
db
.prepare("SELECT COUNT(*) AS n FROM audit_log WHERE path = ? AND owner_id = ?")
.get(`unshare workspace ${laneA}`, users.bob.id).n,
1
);
});
+9
View File
@@ -1873,6 +1873,15 @@ try {
db.prepare("ALTER TABLE dashboard_runs ADD COLUMN pane_token_hash TEXT").run();
}
// Who started a run, when it is not the workspace's owner (a user the
// workspace is shared with opening a Terminal tab): its pane token acts as
// THEM, never as the owner (server/lib/auth.js `userFromPaneToken`). NULL = the owner.
try {
db.prepare("SELECT started_by FROM dashboard_runs LIMIT 1").get();
} catch {
db.prepare("ALTER TABLE dashboard_runs ADD COLUMN started_by INTEGER").run();
}
// A realtime chat run's bridge secret (`AGENTHUB_CHAT_TOKEN`), SHA-256: only the
// pane's own bridge may stream that run's events (server/routes/chat.js).
// Never listed.
+16 -5
View File
@@ -729,25 +729,36 @@ function newPaneToken() {
}
/**
* The workspace owner a request from inside a live pane acts as: its
* The user a request from inside a live pane acts as: its
* `x-agenthub-pane-token` names a run that is still running in tmux, on a
* workspace someone owns, and the request came straight from this machine.
* That user is the workspace owner — or, for a pane a user the workspace is
* shared with started (`dashboard_runs.started_by`), THAT user, and only while
* it is still shared with them: their Terminal tab never acts as the owner.
*/
function userFromPaneToken(req) {
const t = req.headers[PANE_HEADER];
if (typeof t !== "string" || !t.startsWith("ahpane_") || t.length > 200) return null;
if (!isLocalRequest(req)) return null;
const row = sql(
`SELECT r.id, r.lane_id, l.owner_id FROM dashboard_runs r JOIN lanes l ON l.id = r.lane_id
`SELECT r.id, r.lane_id, r.started_by, l.owner_id FROM dashboard_runs r JOIN lanes l ON l.id = r.lane_id
WHERE r.pane_token_hash = ? AND r.status IN ('running', 'spawning') AND instr(r.id, '@') = 0`
).get(sha256(t));
if (!row || row.owner_id == null) return null;
// S3: only from the account that pane runs as (any local uid could post it).
if (!require("./hook-auth").peerIsPaneAccount(req, row.lane_id)) return null;
if (!require("./tmux").hasSession(row.id)) return null;
return (
publicUser(sql("SELECT * FROM users WHERE id = ? AND disabled = 0").get(row.owner_id)) || null
);
const uid = row.started_by ?? row.owner_id;
const user = sql("SELECT * FROM users WHERE id = ? AND disabled = 0").get(uid);
if (!user) return null;
if (
uid !== row.owner_id &&
user.role !== "admin" &&
!require("./scope").sharedWith(row.lane_id, uid)
) {
return null;
}
return publicUser(user) || null;
}
// ── Gates ───────────────────────────────────────────────────────────────────
+3 -2
View File
@@ -34,11 +34,11 @@ const insertStmt = db.prepare(`
INSERT OR REPLACE INTO dashboard_runs (
id, session_id, mode, cwd, model, permission_mode, effort,
resume_session_id, prompt_preview, status, exit_code, started_at, ended_at,
lane_id, tool, worktree, proxy, account, pane_token_hash
lane_id, tool, worktree, proxy, account, pane_token_hash, started_by
) VALUES (
@id, @session_id, @mode, @cwd, @model, @permission_mode, @effort,
@resume_session_id, @prompt_preview, @status, @exit_code, @started_at, @ended_at,
@lane_id, @tool, @worktree, @proxy, @account, @pane_token_hash
@lane_id, @tool, @worktree, @proxy, @account, @pane_token_hash, @started_by
)
`);
@@ -128,6 +128,7 @@ function recordRun(handle) {
proxy: handle.proxy || null,
account: handle.account || null,
pane_token_hash: handle.paneTokenHash || null,
started_by: Number.isInteger(handle.startedBy) ? handle.startedBy : null,
});
} catch (err) {
// Best-effort by design (a live run must never fail because sqlite did),
+4 -1
View File
@@ -808,7 +808,9 @@ function spawnRun(args) {
* `shell` so the tab can say what it is. The caller has already made sure the
* slot holds no live session.
*/
function spawnShell({ laneId, slot = 0, cwd }) {
/** `startedBy`: the user opening it when that is not the workspace's owner (a
* workspace shared with them) — its pane token then acts as them. */
function spawnShell({ laneId, slot = 0, cwd, startedBy = null }) {
if (!Number.isInteger(laneId)) throw makeErr("EBADLANE", "laneId must be an integer");
if (!Number.isInteger(slot) || slot < 0 || slot > MAX_PANE_SLOT) {
throw makeErr("EBADSLOT", `slot must be an integer 0–${MAX_PANE_SLOT}`);
@@ -844,6 +846,7 @@ function spawnShell({ laneId, slot = 0, cwd }) {
worktree: false,
proxy: null,
paneTokenHash: paneCred ? paneCred.hash : null,
startedBy: lane && Number.isInteger(startedBy) && startedBy !== lane.owner_id ? startedBy : null,
prompt: "",
status: "running",
startedAt: Date.now(),
+14 -5
View File
@@ -1416,11 +1416,12 @@ function laneShares(laneId) {
return require("../db")
.db.prepare(
`SELECT u.id, u.username, COALESCE(NULLIF(u.display_name, ''), u.username) AS name,
s.shared_at AS sharedAt
s.shared_at AS sharedAt, u.disabled
FROM lane_shares s JOIN users u ON u.id = s.user_id
WHERE s.lane_id = ? ORDER BY name COLLATE NOCASE`
)
.all(Number(laneId));
.all(Number(laneId))
.map((u) => ({ ...u, disabled: Boolean(u.disabled) }));
}
router.get("/:id/shares", (req, res) => {
@@ -1455,10 +1456,13 @@ router.put("/:id/shares", sameOriginGuard, (req, res) => {
}
const { db } = require("../db");
const wanted = [...new Set(raw)];
const before = new Set(laneShares(lane.id).map((u) => u.id));
const user = db.prepare("SELECT id, disabled FROM users WHERE id = ?");
for (const id of wanted) {
const u = user.get(id);
if (!u || u.disabled) {
// A user already on the list may since have been disabled: keeping them
// must not block every other change (it grants nothing while disabled).
if (!u || (u.disabled && !before.has(id))) {
return res
.status(400)
.json({ error: { code: "EBADUSER", message: `no enabled user ${id}` } });
@@ -1469,7 +1473,6 @@ router.put("/:id/shares", sameOriginGuard, (req, res) => {
.json({ error: { code: "EBADUSER", message: "a workspace is not shared with its owner" } });
}
}
const before = new Set(laneShares(lane.id).map((u) => u.id));
const added = wanted.filter((id) => !before.has(id));
const removed = [...before].filter((id) => !wanted.includes(id));
const actor = req.user?.id ?? null;
@@ -3497,7 +3500,13 @@ router.post("/:id/:action", sameOriginGuard, async (req, res) => {
if (picked === null) return { allBusy: true };
slot = picked;
if (isLive(current.id, slot)) return { conflict: true };
startedRunId = runs.spawnShell({ laneId: current.id, slot, cwd: current.cwd }).id;
startedRunId = runs.spawnShell({
laneId: current.id,
slot,
cwd: current.cwd,
// A user it is shared with: their pane token acts as them.
startedBy: req.user?.id ?? null,
}).id;
return {};
});
if (shellOutcome.missing) {
+3 -2
View File
@@ -618,8 +618,9 @@ router.post("/", async (req, res) => {
if (!Number.isInteger(laneId)) {
return res.status(400).json({ error: { code: "EBADLANE", message: "laneId is required" } });
}
// A workspace shared with this user: no AI session of theirs yet (sharing phase 1).
if (lanesLib.getLane(laneId) && !scope.isLaneOwner(req, laneId) && body.shell !== true) {
// A workspace shared with this user: no AI session of theirs yet (sharing
// phase 1). This route always starts the lane's AI tool — no shell here.
if (lanesLib.getLane(laneId) && !scope.isLaneOwner(req, laneId)) {
return require("./lanes").collabStart(res);
}
let cwd;