Repository navigation
Commit 3a054cb
feat(spec,runtime,plugin-audit): an action run through MCP leaves an audit row naming its surface (#28171)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_017ihL2fi9zfJhUTubyJYb6V
Fixes #27908
Clause-②: yes (widening)
Metadata-protocol: added
## 维护者速读
- **改了什么:** 每次通过 REST 动作门(控制台按钮、SDK)或 MCP(AI agent 的 `run_action` /
`resume_run`)调用业务动作,审计日志 `sys_audit_log` 都多一行
`action_invocation`,`metadata.surface` 写明 `rest` 还是 `mcp`;agent
代人调用时同一行还带 `performed_by` / `on_behalf_of`。四个分支(脚本、api
转发、flow、声明式更新)都写,成功和分派后被拒都写。
- **为什么改:** 执行 #27681 裁决 6513869839 参数 3:管理员要在审计日志里分清"agent
调的"和"人点的",不需要第二份日志。之前只有脚本分支打一条 `console.info`,其余三个分支什么都不留。
- **风险与回滚:** 只新增审计写入,不改授权、不改哪个门能跑哪个动作;审计写失败只打一次 `error`
日志,动作本身的结果不变。回滚即回退本 PR,已写入的审计行留在账本里。
- **先看:** ① `packages/spec/src/kernel/execution-context.zod.ts` 新键
`surface`(取值 `rest` / `mcp`);②
`packages/runtime/src/action-invocation-audit.ts`(门如何盖上自己的 surface、为何
api 转发不需要额外的头);③
`packages/plugins/plugin-audit/src/action-invocation-audit.ts`(审计行的形状)。
- **两分钟验证:** `pnpm --filter @objectstack/runtime exec vitest run
src/action-invocation-audit.test.ts`,预期 `Tests 24 passed`:四个分支 ×
两个门各写一行,伪造的转发头和上游 `surface` 都改不了记录的门。
- **需要你做的:** 批准键名 `ExecutionContext.surface` 及其取值 `rest` / `mcp`(连同
`sys_audit_log.action` 新值 `action_invocation`)。
Independent review: none run (quota); the PR waits for the seat's
contract-review hand-off.
## What changes
**The key — `ExecutionContext.surface`** (`@objectstack/spec`).
`z.enum(['rest', 'mcp']).optional()`, with the `performedBy` docblock
discipline (ADR-0090 D10 rule 4): set only by the two action doors, from
the door's own identity, on the envelope of the invocation they record;
never client-supplied; attribution only (no middleware reads it, never
an authorization subject, scope or gate). The enum is closed to the
doors that invoke an action today: `rest` (REST `POST
/actions/:object/:action`) and `mcp` (the `/mcp` bridge's `run_action` /
`resume_run`). No other door invokes a business action: flow action
nodes and the declarative endpoint executor do not route through either
door (measured: `invokeBusinessAction` has one caller,
`domains/mcp.ts`). **The maintainer approves the key name and its values
in this PR.** `@objectstack/core` lists it as a non-entry field
(identity resolution does not produce it; the same credential reaches
both doors).
**The doors** (`@objectstack/runtime`). `action-invocation-audit.ts`
holds the door half: `withActionSurface(ec, surface)` (a COPY of the
caller's envelope with the surface overwritten, so the request's own
envelope is unchanged and nothing upstream can move the value) and
`recordActionInvocation`, which resolves the `audit` service slot and
hands it one event. Each door's post-gate dispatch is split into its own
function, code unchanged, so the door can write the row for whatever
that dispatch answers:
- REST: `handleActionsRequest` → `dispatchAdmittedRestAction`; the row
reads the answered status (and `error.code`).
- MCP `run_action`: `invokeBusinessAction` →
`dispatchAdmittedBusinessAction`; the row reads the thrown error through
`resolveThrownHttpError`.
- MCP `resume_run`: `resumeActionRun` → `continueAdmittedRun` (steps
4–6); the row names the admitting flow action and carries the `run_id`.
**Which calls write a row, and why.** Every call that cleared its door's
admission gates and reached its branch: the branch's success, and every
refusal from there on (the subject-record verdict, a declarative write
the data plane refused, an api target's non-2xx, the api loop refusal, a
flow run that failed, a body that threw, "no handler registered"). A
call refused by an admission gate (not found, the `sys_*` / `ai.exposed`
gates, ADR-0066 D4 `requiredPermissions`, the ADR-0126 activation
switch, the param contract, the confirmation gate, and for `resume_run`
steps 1–3) ran nothing and writes no row: the ruling asks to tell an
agent's run from a click, and those calls ran nothing.
**The row** (`@objectstack/plugin-audit`).
`createActionInvocationAuditWriter` joins `createAuthEventAuditSink` on
the `audit` slot (`recordActionInvocation`). It fits `sys_audit_log`
with **no new field**: `action: 'action_invocation'` (a new value of the
existing `action` select, added writer-first;
`sys-audit-log-retired-actions.test.ts` names its writer), `user_id` /
`actor` / `tenant_id` from the envelope by the CRUD mirror's rules,
`object_name` + `record_id` for the subject (so the ledger's record read
gate judges it), no snapshots and no params, and `metadata` = `{
surface, action_name, branch, outcome, status?, error_code?, run_id?,
performed_by?, on_behalf_of? }`. A lost row is reported once per cause
at `error` (`persistActionInvocationAuditRow` is registered in
`DURABILITY_CRITICAL_CALLEES`). The `[action-audit]` log line is kept
and is not the row.
**The api forward.** The row on the `api` branch is written by the door
after the in-process forward answers, from the door's own envelope,
which the target's re-authentication never replaces; so it names the
true door with no surface header on the forward (pinned: the forwarded
request carries none, and the row says `mcp`). See Acceptance notes for
why no trusted forward header was added.
## Tests
- `packages/runtime/src/action-invocation-audit.test.ts` (24): every
branch × both doors (REST through `HttpDispatcher.handleActions`, MCP
through the real `buildMcpBridge`) hands the slot exactly one event with
the right `surface`, `branch`, `outcome`; a refusal after dispatch on
every branch × both doors records `failed` with the door's status and
code; the subject-record 404 writes one failed row per door; a D4-gated
call writes none; the forged-header case (an outside REST request
carrying the forward mark, an `x-objectstack-action-surface: mcp` header
and an upstream `surface: 'mcp'` still records `rest`; an MCP call
arriving as `surface: 'rest'` still records `mcp`); the api row after
the forward; the request envelope left unchanged; the agent's
`performedBy` beside the surface; `resume_run` success and refusal rows.
- `packages/plugins/plugin-audit/src/action-invocation-audit.test.ts`
(7): through a real ObjectQL engine, a click and an agent's call of the
same action land as two rows that differ by `surface` and
`performed_by`; a refusal's status/code and a resume's run id;
object-less actions; no guessed surface; the `actor`-less ledger; the
once-per-cause `error` report with no throw.
- `sys-audit-log-retired-actions.test.ts`: `action_invocation` added to
the writer inventory. `assemble-execution-context.test.ts`: `surface`
added to the non-entry mirror.
**Ablation** (one-shot, `scripts/ablation-replace.mjs`, mutation proven
on disk, restored by blob hash):
- runtime: `await sink.recordActionInvocation(event);` → removed ⇒
`action-invocation-audit.test.ts` **20 failed | 4 passed** (the 4 are
the no-row gate case and the helper cases); restored blob
`d014ccafe5bb`.
- plugin-audit: `await persistActionInvocationAuditRow(engine, row);` →
removed ⇒ **3 failed | 4 passed** (every read-back case); restored blob
`9d2076906454`.
## Acceptance notes
- **Metadata-protocol: `added`, not the claim's `none`** (the order
asked to check `pr-automation.yml`'s WHICH LEVEL table and follow it).
The table's gate (`check-changeset-no-major.mjs`, `none-baseline-moved`)
refuses `none` when an `authorable-surface/` shard moves, and this key
moves `packages/spec/authorable-surface/kernel.json`
(`kernel/ExecutionContext:surface`, regenerated by the build — the shard
lists every `ExecutionContext` key). So the declaration is `added` and
every moved package's changeset is `minor` (spec, core, runtime,
plugin-audit).
- **No trusted surface header on the api forward** (deviation from the
order's suggested route; the seat decides). Measured: (1) the row does
not need it, as above; (2) the forward mark the PM assumption leans on
carries no trust — any outside caller can send
`x-objectstack-action-forward`, and its only effect is the loop refusal
— so a surface header "honoured with the same trust" would be honoured
from any caller, letting an agent's call record `rest`; (3) a sound
carry needs a new in-process credential, and each face that resolves an
envelope (`resolveRequestExecutionContext` has seven door callers) would
have to honour it; (4) no far-side writer would read it: the CRUD
mirror's hook provenance carries `flowRunId` / `attributedUserId` /
`performedByClientId` only, and the `sys_*` identity targets write
through better-auth's adapter under its own system context. Carrying the
surface onto the CRUD rows of the target's writes is a separate change
(HookContext provenance + objectql), not made here.
- `sys_audit_log` gains an enum VALUE, not a field. `tenant_id` is the
caller's active organization (`ExecutionContext.tenantId`); the CRUD
mirror prefers the record's own organization, which this row does not
read.
- Read from code, not measured: an OAuth-token agent's `api` action
forward reaches its target with the bearer the dispatcher honours only
on `/mcp` paths (`acceptOAuthAccessToken` path match,
`http-dispatcher.ts`), so the target would resolve the caller as
anonymous. Out of this card; reported to the seat.
## Verification
<details><summary>Local gates and tests (head and commands in the
os-dev-report comment on #27908)</summary>
- `pnpm build` (whole workspace, under the verify lock): exit 0
- `@objectstack/spec check:generated`: 14/14 up to date
- typecheck spec / core / runtime / plugin-audit: exit 0
- tests: core 128 files / 2873 passed; plugin-audit 66 / 1058; runtime
484 / 7895 passed, 19 skipped; spec 756 / 22510 passed, 1 todo;
`runtime/src/action-invocation-audit.test.ts` 24 / 24 on the final head
- `node scripts/pm/dispatch-gates.mjs --commands`: 131 derived commands,
each run with its exit code recorded before any pipe; `--ran`
reconciliation: 131 derived, 131 run, 0 NOT-MEASURED, 0 UNRUN, every
exit 0
- The first pass was red on six commands (five gates), all fixed in the
last commit: `check-tenant-audit-census` (the new write site; the
options cast is dropped so the census reads its system context, and the
page is recounted 251 to 252), `check:engine-double-contract` (ledger
`--write`), `check:objectql-double-limit` (the test double now applies
`limit`), `check:swallow-census-controls` (the new durability callee
copied into `WRITE_SHAPED_CALLEES`), `check:doc-authoring` (issue id
dropped from the key's `.describe()`). Three more first answered exit 3
(not measured: no build or no lock turn) and were measured green after
`pnpm build`: `check:skill-examples`, `check:dual-build-cjs-loads`,
`check:type-check-debt`.
- `check-adr-0087-registration.mjs --base origin/main`: exit 0 (no
declared-breaking changeset)
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ihL2fi9zfJhUTubyJYb6V)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent a0f8654 commit 3a054cb
30 files changed
Lines changed: 1314 additions & 56 deletions
File tree
- .changeset
- content/docs
- permissions
- references
- data
- kernel
- docs/audits
- packages
- core/src/security
- plugins/plugin-audit/src
- objects
- translations
- runtime/src
- domains
- spec
- authorable-surface
- src/kernel
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
132 | 132 | | |
133 | 133 | | |
134 | 134 | | |
135 | | - | |
| 135 | + | |
136 | 136 | | |
137 | 137 | | |
138 | 138 | | |
| |||
197 | 197 | | |
198 | 198 | | |
199 | 199 | | |
200 | | - | |
| 200 | + | |
201 | 201 | | |
202 | | - | |
203 | | - | |
| 202 | + | |
| 203 | + | |
204 | 204 | | |
205 | 205 | | |
206 | 206 | | |
| |||
217 | 217 | | |
218 | 218 | | |
219 | 219 | | |
220 | | - | |
| 220 | + | |
221 | 221 | | |
222 | 222 | | |
223 | 223 | | |
224 | | - | |
| 224 | + | |
225 | 225 | | |
226 | 226 | | |
227 | 227 | | |
| |||
233 | 233 | | |
234 | 234 | | |
235 | 235 | | |
236 | | - | |
237 | | - | |
| 236 | + | |
| 237 | + | |
238 | 238 | | |
239 | | - | |
| 239 | + | |
240 | 240 | | |
241 | | - | |
| 241 | + | |
242 | 242 | | |
243 | 243 | | |
244 | 244 | | |
245 | 245 | | |
246 | | - | |
| 246 | + | |
247 | 247 | | |
248 | 248 | | |
249 | 249 | | |
250 | 250 | | |
251 | 251 | | |
252 | | - | |
| 252 | + | |
253 | 253 | | |
254 | 254 | | |
255 | 255 | | |
256 | 256 | | |
257 | | - | |
| 257 | + | |
258 | 258 | | |
259 | 259 | | |
260 | 260 | | |
| |||
307 | 307 | | |
308 | 308 | | |
309 | 309 | | |
310 | | - | |
| 310 | + | |
311 | 311 | | |
312 | 312 | | |
313 | 313 | | |
314 | | - | |
| 314 | + | |
315 | 315 | | |
316 | | - | |
317 | | - | |
| 316 | + | |
| 317 | + | |
318 | 318 | | |
319 | 319 | | |
0 commit comments