fix: use unified identity for bash ownership

This commit is contained in:
Tianyi Cui
2026-07-14 07:26:46 +08:00
parent c2979322c8
commit 99b5f04c9b
8 changed files with 40 additions and 47 deletions

View File

@@ -37,7 +37,7 @@ interface BashExecRequest {
env?: Record<string, string> | undefined
/**
* Opaque OWNER token for a background task — the consumer's isolation key
* (the tool layer passes the owning agent's `session.header.id`). The
* (the tool layer passes the owning agent's shared `id`). The
* executor stores it on the task and exposes it via {@link BashExecutor.ownerOf};
* the executor itself NEVER interprets it (no access policy lives in the
* seam — that is the consumer's job). Absent for foreground runs and for an

View File

@@ -22,7 +22,7 @@ A new `cancel()` verb on the `Agent` interface — the single public stop primit
### 3. Bash owner token in the seam
Background-task ownership moved from a `tool-bash` plugin-local `Map<string, Agent>` into the executor. `BashExecRequest` gains an optional `owner?: string`; the resolved `BashExecSpec` carries it as required-but-nullable `owner: string | undefined` (a forgotten owner is a visible `undefined`, never a silently-absent property). The executor stores the token on its task and exposes it via a new `BashExecutor.ownerOf(id): string | undefined` seam (NOT on the public `BashTask` — one read path, no redundant API). `tool-bash` deletes its `Map` entirely: it stamps `exec.agent?.session.header.id` as the owner at `start`, and `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's token with `!== undefined` semantics (an empty-string token is still a real owner). The completion notice finds the live agent by scanning `ctx.get('agents')?.list()` for `agent.session.header.id === ownerToken` (read via `ctx.get``onTaskDone` runs on the bash fiber, a foreign fiber, where the `ctx.agents` proxy would throw). Because ownership now lives on the task in the executor (disposed with the `dsh-bash` fiber), it SURVIVES a `tool-bash` HMR reload — closing the old `XXX(tool-bash-owner-hmr)` gap. (The `onTaskDone` listener is still effect-scoped to `tool-bash`'s `apply`, so a completion landing during the reload gap still drops its one notice — the pre-existing reload-gap drop — but the ownership fence itself is HMR-proof.)
Background-task ownership moved from a `tool-bash` plugin-local `Map<string, Agent>` into the executor. `BashExecRequest` gains an optional `owner?: string`; the resolved `BashExecSpec` carries it as required-but-nullable `owner: string | undefined` (a forgotten owner is a visible `undefined`, never a silently-absent property). The executor stores the token on its task and exposes it via a new `BashExecutor.ownerOf(id): string | undefined` seam (NOT on the public `BashTask` — one read path, no redundant API). `tool-bash` deletes its `Map` entirely: it stamps `exec.agent?.id` (the shared registry/session id) as the owner at `start`, and `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's token with `!== undefined` semantics (an empty-string token is still a real owner). The completion notice finds the live agent by scanning `ctx.get('agents')?.list()` for `agent.id === ownerToken` (read via `ctx.get``onTaskDone` runs on the bash fiber, a foreign fiber, where the `ctx.agents` proxy would throw). Because ownership now lives on the task in the executor (disposed with the `dsh-bash` fiber), it SURVIVES a `tool-bash` HMR reload — closing the old `XXX(tool-bash-owner-hmr)` gap. (The `onTaskDone` listener is still effect-scoped to `tool-bash`'s `apply`, so a completion landing during the reload gap still drops its one notice — the pre-existing reload-gap drop — but the ownership fence itself is HMR-proof.)
## Verification
@@ -35,7 +35,7 @@ These invariants hold and are pinned by tests:
## Session owner tokens are unique among live agents
The bash owner-token comparison relies on `session.header.id` being unique among live agents. Concurrent same-ID operations may both prepare privately, but publication enters the session and agent in order; `SessionStore.enter()` rejects a duplicate live session id, and every losing transaction rolls its private state back. A programmatic caller therefore cannot publish two live agents with one session token. The access *policy* (token comparison) stays in `tool-bash` (the consumer); the bash seam stores only an opaque `owner` string and never interprets it — the correct interface/implementation/consumer split.
The bash owner-token comparison relies on the shared `Agent.id`/`SessionId` being unique among live agents. Concurrent same-ID operations may both prepare privately, but publication enters the session and agent in order; `SessionStore.enter()` rejects a duplicate live session id, and every losing transaction rolls its private state back. A programmatic caller therefore cannot publish two live agents with one session token. The access *policy* (token comparison) stays in `tool-bash` (the consumer); the bash seam stores only an opaque `owner` string and never interprets it — the correct interface/implementation/consumer split.
## Alternatives considered

View File

@@ -8,7 +8,7 @@ The harness brands `CallId` (`packages/llm/llm/src/brand.ts`) and the shared age
**Gap 1 — unbranded cross-boundary IDs in the bash seam.** The background-task id is a plain `string`: `BashTask.id: string` (`packages/bash/bash/src/types.ts`), carried as `string` through the whole executor seam (`BashExecutor.get`/`ownerOf`/`readOutput`/`kill(id: string)` in `packages/bash/bash/src/index.ts`) and validated/passed as `string` by the model-facing tools (`validateTaskId`, `assertTaskAccess`, the `task_id` schema arg in `packages/bash/tool-bash/src/index.ts`). It is generated by a per-executor counter — `` `bash-${this.nextTaskId++}` `` in `packages/bash/bash-local/src/index.ts` — which gives it **exactly the same `name-N` shape as `SessionId`'s default** (`` `session-${++counter}` `` in `packages/core/session/src/index.ts`). A bash task id and a session id are trivially swappable at a call site and the compiler says nothing. This is the headline case the user asked about, and it is a model-facing id (the model passes `task_id` back to `bash_output`/`bash_kill`), so a confusion here is reachable from untrusted input.
The bash **owner token** is the related sub-case: `BashExecRequest.owner?: string` and `BashExecSpec.owner: string | undefined` (`packages/bash/bash/src/types.ts`) are documented as a deliberately *opaque* isolation key, but in every live caller the value IS the owning agent's `session.header.id` (`callerToken = (exec) => exec.agent?.session.header.id` in `packages/bash/tool-bash/src/index.ts`) — i.e. a `SessionId` wearing a `string` disguise. It is compared for access control (`owner !== callerToken(exec)`), so a mismatched-but-well-typed string here is a cross-session isolation bug the type system currently cannot catch. This is the same `session.header.id`-as-owner alias covered by the [unified agent/session identity decision](../simplification/2026-06-20-unify-agent-and-session-id.md).
The bash **owner token** is the related sub-case: `BashExecRequest.owner?: string` and `BashExecSpec.owner: string | undefined` (`packages/bash/bash/src/types.ts`) are documented as a deliberately *opaque* isolation key, but in every live caller the value IS the owning agent's shared `Agent.id`/`SessionId` (`callerToken = (exec) => exec.agent?.id` in `packages/bash/tool-bash/src/index.ts`) wearing a different seam-local name. It is compared for access control (`owner !== callerToken(exec)`), so a mismatched-but-well-typed string here is a cross-session isolation bug the type system currently cannot catch. This is the shared id alias covered by the [unified agent/session identity decision](../simplification/2026-06-20-unify-agent-and-session-id.md).
**Gap 2 — brand erosion at the seams of the *already-branded* IDs.** Even `CallId` and `SessionId` decay back to bare `string` at exactly the places confusion is most likely: registry/store key types and public method params. Representative sites include the session store, the agent registry (both keyed by the shared `SessionId`), `ToolPresenter`'s call-id map, ACP's session-id records and loading set, and the persistence coordinator. A brand that is dropped at a collection key buys nothing on lookups — the value of the existing brands is partly unrealized.
@@ -18,7 +18,7 @@ A type-only change. Brands are zero-cost casts; nothing about runtime behavior,
- **Brand the bash task id.** Add `BashTaskId = Branded<'BashTaskId'>` plus its same-named factory in `packages/bash/bash/src/types.ts` (the package that *owns* the id), importing `Branded` from `@deepseek-ai/dsh-brand` exactly as `SessionId` does. The brand primitive lives in the dependency-free `dsh-brand` utility package precisely so `dsh-bash` can brand its ids by depending on it alone — it never pulls in `dsh-llm` (or `dsh-session`) just to reach `Branded`. Thread it through `BashTask.id`, the `BashExecutor` seam methods (`get`/`ownerOf`/`readOutput`/`kill`), the generation site in `dsh-bash-local` (brand the counter output once, at creation), and the `dsh-tool-bash` validate/access surface (`validateTaskId` returns a `BashTaskId`; `task_id` is branded at the tool boundary where the model's string arrives).
- **Mint a distinct `OwnerToken` brand.** Add `OwnerToken = Branded<'OwnerToken'>` in `packages/bash/bash/src/types.ts`; type `BashExecRequest.owner` / `BashExecSpec.owner` / `BashExecutor.ownerOf` as `OwnerToken | undefined`. The `dsh-tool-bash` consumer casts the agent's `session.header.id` (a `SessionId`) into an `OwnerToken` at the boundary — the one place the two vocabularies meet. The bash seam never imports `dsh-session`. (Rationale in the next section.)
- **Mint a distinct `OwnerToken` brand.** Add `OwnerToken = Branded<'OwnerToken'>` in `packages/bash/bash/src/types.ts`; type `BashExecRequest.owner` / `BashExecSpec.owner` / `BashExecutor.ownerOf` as `OwnerToken | undefined`. The `dsh-tool-bash` consumer casts the agent's shared `id` (`SessionId`) into an `OwnerToken` at the boundary — the one place the two vocabularies meet. The bash seam never imports `dsh-session`. (Rationale in the next section.)
- **Stop the brand erosion.** Propagate the existing brands to the `Map` key types and public method params listed under Gap 2 — `Map<SessionId, Session>`, `Map<SessionId, Agent>`, `get(id: SessionId)`, `Map<CallId, …>`, ACP's `SessionId` surface, and the coordinator's `Map<SessionId, …>`. This is the larger mechanical share of the diff and the part that makes the *existing* brands actually load-bearing on lookups, not just on struct fields.

View File

@@ -117,7 +117,7 @@ export interface BashExecRequest {
env?: Record<string, string> | undefined
/**
* Opaque OWNER token for a background task — the consumer's isolation key
* (the tool layer passes the owning agent's `session.header.id`). The
* (the tool layer passes the owning agent's shared `id`). The
* executor stores it on the task and exposes it via {@link BashExecutor.ownerOf};
* the executor itself NEVER interprets it (no access policy lives in the
* seam — that is the consumer's job). Absent for foreground runs and for an

View File

@@ -34,7 +34,7 @@ Result text: stdout, then a `[stderr]` section, then status markers — `[sandbo
### Task ownership (cross-session isolation)
The owning agent's session token (`session.header.id`) is stamped onto the task at spawn — passed to the executor via `resolve({ …, owner })` and stored ON THE TASK inside the executor (the `dsh-bash` `ownerOf(id)` seam), **not** in a plugin-local map. `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's token (`session.header.id`) with `!== undefined` semantics and reject a task owned by a *different* session with `task <id> belongs to another session` (a task started with no agent — a non-loop caller — has no owner token and is open to anyone; a call with no `exec.agent` cannot access an owned task). Task ids are global and predictable, so under multi-session ACP this token check is the fence that stops one session's agent from reading or killing another session's background task. Because ownership lives on the task in the executor (disposed with the `dsh-bash` fiber), it **survives an independent `tool-bash` HMR reload** — closing the old plugin-local-map gap where a reload orphaned pre-reload tasks. (The `onTaskDone` listener is still effect-scoped to this plugin's `apply`, so a completion landing during the reload gap still drops its one notice — the pre-existing reload-gap drop — but the ownership fence itself is HMR-proof.)
The owning agent's shared registry/session id (`agent.id`) is stamped onto the task at spawn — passed to the executor via `resolve({ …, owner })` and stored ON THE TASK inside the executor (the `dsh-bash` `ownerOf(id)` seam), **not** in a plugin-local map. `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's shared id with `!== undefined` semantics and reject a task owned by a *different* session with `task <id> belongs to another session` (a task started with no agent — a non-loop caller — has no owner token and is open to anyone; a call with no `exec.agent` cannot access an owned task). Task ids are global and predictable, so under multi-session ACP this token check is the fence that stops one session's agent from reading or killing another session's background task. Because ownership lives on the task in the executor (disposed with the `dsh-bash` fiber), it **survives an independent `tool-bash` HMR reload** — closing the old plugin-local-map gap where a reload orphaned pre-reload tasks. (The `onTaskDone` listener is still effect-scoped to this plugin's `apply`, so a completion landing during the reload gap still drops its one notice — the pre-existing reload-gap drop — but the ownership fence itself is HMR-proof.)
## UI presentation
@@ -42,7 +42,7 @@ These tools own how their calls render in a UI (an editor's tool-call card) via
## Background completion notices
When a background task finishes, a short notice is injected into the owning agent's session (`agent.inject()`, source `{kind: 'plugin', plugin: 'tool-bash'}`). The owning agent is found by its session token: the listener reads `ctx.bash.ownerOf(task.id)` and scans `ctx.get('agents')?.list()` for an agent whose `session.header.id` matches (read via `ctx.get``onTaskDone` runs on the bash fiber, a foreign fiber, so the `ctx.agents` proxy would throw). If no live agent carries that token — e.g. the owning session disconnected and its agent was disposed while the task ran on — the notice is dropped cleanly. Injection is **durable context for the next model request, not a wake-up** — an idle agent stays idle until something sends a message. That's why the tool descriptions tell the model to poll with `bash_output`.
When a background task finishes, a short notice is injected into the owning agent's session (`agent.inject()`, source `{kind: 'plugin', plugin: 'tool-bash'}`). The listener reads `ctx.bash.ownerOf(task.id)` and scans `ctx.get('agents')?.list()` for that shared agent/session id (read via `ctx.get``onTaskDone` runs on the bash fiber, a foreign fiber, so the `ctx.agents` proxy would throw). If no live agent carries that token — e.g. the owning session disconnected and its agent was disposed while the task ran on — the notice is dropped cleanly. Injection is **durable context for the next model request, not a wake-up** — an idle agent stays idle until something sends a message. That's why the tool descriptions tell the model to poll with `bash_output`.
## The tool builds its request from named args only

View File

@@ -12,7 +12,7 @@
* `bash_output`.
*
* Task ownership: a background task's OWNER is an opaque token — the owning
* agent's `session.header.id` — passed to the executor at spawn
* agent's shared `id` — passed to the executor at spawn
* (`resolve({ …, owner })`) and stored ON THE TASK inside the executor
* (`@deepseek-ai/dsh-bash`'s `ownerOf(id)` seam), NOT in a plugin-local map.
* `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's token
@@ -414,16 +414,12 @@ export function apply(ctx: Context): void {
})
/**
* The caller's owner TOKEN — the owning agent's `session.header.id`, or
* `undefined` for a non-agent caller. Read `session.header.id` (NOT
* `session.id`): every other subsystem keys off the header id (the ACP bridge,
* both persistence backends), and the sibling `resolveWorkdir` already reads
* `session.header.cwd`, so using `session.id` here would be the asymmetry smell
* the conventions flag. The two are equal in production, but the header is the
* canonical identity.
* The caller's owner TOKEN — the owning agent's shared registry/session id,
* or `undefined` for a non-agent caller. Agent and Session deliberately have
* one live identity; workdir remains separate session metadata.
*/
const callerToken = (exec: { agent?: Agent }): OwnerToken | undefined =>
exec.agent ? OwnerToken(exec.agent.session.header.id) : undefined
exec.agent ? OwnerToken(exec.agent.id) : undefined
/**
* Authorize a `bash_output`/`bash_kill` call against the task's stored owner
@@ -443,18 +439,16 @@ export function apply(ctx: Context): void {
}
// Background completion → inject a notice into the owning agent's session.
// Find the live agent by its session id token via the agent registry, read
// Find the live agent by its shared registry/session token, read
// opportunistically with `ctx.get('agents')` (NOT `ctx.agents`/static inject):
// this listener runs from `task.done.then` on the bash fiber — a foreign
// fiber — where the `ctx.agents` property proxy would throw through the
// traceable shadow; `ctx.get(name)` is the topology-independent lookup. No
// registry mounted (`undefined`) → drop the notice. Match on
// `agent.session.header.id`, NOT the registry key: a config agent's id differs
// from its session id, and the owner token IS the session id.
// registry mounted (`undefined`) → drop the notice.
ctx.bash.onTaskDone((task) => {
const ownerToken = ctx.bash.ownerOf(task.id)
if (ownerToken === undefined) return
const agent = ctx.get('agents')?.list().find(a => OwnerToken(a.session.header.id) === ownerToken)
const agent = ctx.get('agents')?.list().find(a => OwnerToken(a.id) === ownerToken)
if (!agent) return
try {
agent.inject(

View File

@@ -47,22 +47,16 @@ async function setup() {
}
/**
* Build a fake {@link Agent} whose session token is `sessionId`, REGISTER it in
* `ctx.agents` (the completion-notice path finds the owning agent by scanning
* the registry for a matching `session.header.id`), and return it. The returned
* Build a fake {@link Agent} with the shared registry/session `sessionId`,
* REGISTER it in `ctx.agents`, and return it. The returned
* agent is also passed to `execute` as `exec.agent` so it owns the spawned task.
* The registration disposer is tracked so {@link unregisterFakeAgents} can drop
* it (simulating the owning session disconnecting before a task completes).
*/
const fakeAgentDisposers = new Map<Context, (() => Promise<void> | void)[]>()
function registerFakeAgent(ctx: Context, sessionId: string, inject: (...args: unknown[]) => void): Agent {
// The registry KEY (agent.id) is deliberately DIFFERENT from the session
// token (session.header.id), which is also the agent's durable id. The
// owner token IS the session id, so the notice path must find the agent by
// `session.header.id`, NOT the registry key. Using distinct values here makes
// the test fail if a regression matched on the wrong field (a same-value fake
// would pass either way — the "hits the line but not the scenario" trap).
const agent = { id: `agent-${sessionId}`, inject, session: { header: { version: 0, id: sessionId, createdAt: 0 } } } as unknown as Agent
const id = SessionId(sessionId)
const agent = { id, inject, session: new Session(id) } as unknown as Agent
const dispose = ctx.agents.register(agent)
const list = fakeAgentDisposers.get(ctx) ?? []
list.push(dispose)
@@ -418,9 +412,9 @@ describe('background tools', () => {
it('injects a completion notice into the owning agent (found via the registry by session token)', async () => {
const ctx = await setup()
const inject = vi.fn()
// The notice path looks the agent up in ctx.agents by its session token, so
// The notice path looks the agent up in ctx.agents by its shared id, so
// the agent must be REGISTERED (not merely passed to execute). Mount a
// registry and register a fake whose session.header.id IS the owner token.
// registry and register a fake whose agent/session id IS the owner token.
const agent = registerFakeAgent(ctx, 'bg', inject)
const started = await ctx.tools.execute({
@@ -517,13 +511,14 @@ describe('background task ownership (cross-session isolation)', () => {
function callAs(ctx: Context, agent: import('@deepseek-ai/dsh-agent').Agent | undefined, name: string, args: unknown) {
return ctx.tools.execute({ callId: CallId(`own-${++callCounter}`), name, arguments: args, ...agent ? { agent } : {} })
}
// Ownership is by TOKEN (session.header.id), NOT agent object identity — so
// each agent needs a DISTINCT session id, else every fake yields the same
// Ownership is by the shared agent/session TOKEN, NOT agent object identity —
// so each agent needs a DISTINCT id, else every fake yields the same
// token and the isolation tests pass for the wrong reason (all tasks owned by
// the same token). The impl reads `session.header.id`, so the fakes MUST carry
// it.
const fakeAgent = (sessionId: string) =>
({ inject: () => undefined, session: { header: { version: 0, id: sessionId, createdAt: 0 } } }) as unknown as import('@deepseek-ai/dsh-agent').Agent
// the same token).
const fakeAgent = (sessionId: string) => {
const id = SessionId(sessionId)
return { id, inject: () => undefined, session: new Session(id) } as unknown as import('@deepseek-ai/dsh-agent').Agent
}
it('rejects bash_output/bash_kill for a task owned by a DIFFERENT session token', async () => {
const ctx = await setup()
@@ -548,7 +543,7 @@ describe('background task ownership (cross-session isolation)', () => {
})
it('a DIFFERENT Agent object with the SAME session token may access the task (ownership is by token, not object identity)', async () => {
// Ownership fences by session.header.id, NOT Agent object identity. Two
// Ownership fences by the shared id, NOT Agent object identity. Two
// distinct Agent objects sharing one session token (e.g. an agent re-created
// on the same session) are the SAME owner.
const ctx = await setup()
@@ -638,8 +633,10 @@ describe('session-cwd routing (per-session workdir)', () => {
return ctx.tools.execute({ callId: CallId(`cwd-${++callCounter}`), name: 'bash', arguments: args, ...agent ? { agent } : {} })
}
// An agent whose session header carries a cwd (what session/new records).
const agentInCwd = (cwd: string) =>
({ inject: () => undefined, session: { header: { version: 0, id: 'c', createdAt: 0, cwd } } }) as unknown as import('@deepseek-ai/dsh-agent').Agent
const agentInCwd = (cwd: string) => {
const id = SessionId('c')
return { id, inject: () => undefined, session: { header: { version: 0, id, createdAt: 0, cwd } } } as unknown as import('@deepseek-ai/dsh-agent').Agent
}
it('defaults bash to the agent\'s session cwd (not the server launch dir)', async () => {
const ctx = await setup()
@@ -1188,10 +1185,11 @@ describe('sandbox escalation (sandbox_permissions / justification)', () => {
* enforces the enclosure.
*/
function escalationAgent(events: Array<{ type: string; data: Record<string, unknown> }>): Agent {
const id = SessionId('sess-esc')
return {
id: 'agent-esc',
id,
session: {
header: { version: 0, id: 'sess-esc', createdAt: 0 },
header: { version: 0, id, createdAt: 0 },
events: [{ type: 'turn/start' }],
append: (type: string, data: Record<string, unknown>) => { events.push({ type, data }) },
},
@@ -1406,7 +1404,7 @@ describe('per-session sandbox mode (the bash/sandbox-mode fold)', () => {
session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
const injected: string[] = []
const agent = {
id,
id: SessionId(id),
session,
inject: (content: { type: string; text: string }[]) => { injected.push(content[0]?.text ?? '') },
} as unknown as Agent

View File

@@ -69,10 +69,11 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [
'async create(options: CreateAgentOptions): Promise<AgentHandle>',
'async resume(options: ResumeAgentOptions): Promise<AgentHandle>',
'register(agent: Agent): () => void',
'enter(agent: Agent): () => void',
'enter(agent: Agent, owner: Agent | undefined): () => void',
'announce(agent: Agent): void',
'get(id: SessionId): Agent | undefined',
'list(): Agent[]',
'roots(): Agent[]',
],
},
{