Skip to content

feat(plugins): expose requester context to tool hooks#111190

Merged
steipete merged 4 commits into
mainfrom
feat/file-authorization-hook
Jul 19, 2026
Merged

feat(plugins): expose requester context to tool hooks#111190
steipete merged 4 commits into
mainfrom
feat/file-authorization-hook

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Related: #103780

What Problem This Solves

Operators can authorize several people to talk to one agent, but before_tool_call hooks cannot currently distinguish the requester who initiated a run. That prevents a small deployment-local policy file from giving owners broad access while constraining maintainers to safer tools and message actions.

Why This Change Was Made

Expose host-derived channel, account, sender, owner, and provider-role facts through the existing ctx.requester hook context. The same context now reaches managed tools and Codex-native PreToolUse calls. No new authorization configuration schema is added; documentation shows one standalone plugin file that owns the policy and an existing agent binding that routes /fix to the protected agent.

This is the minimal replacement for the broad experimental prototype in #103780. That draft remains explicitly DO NOT LAND.

User Impact

Plugin authors and operators can enforce per-requester tool policy in one TypeScript file. Requester facts are optional and fail-closed examples treat missing identity as unproven. Existing channel admission, sandbox, approval, and owner-only restrictions still apply and cannot be bypassed by the hook.

Evidence

  • Focused managed/native hook suites: 180 tests passed.
  • pnpm plugin-sdk:api:check: passed.
  • pnpm plugin-sdk:surface:check: passed with the public export budget unchanged.
  • pnpm check:changed: passed, including core/extension typecheck and lint lanes.
  • pnpm build: passed.
  • Fresh autoreview: no accepted/actionable findings after fixing channel/account scoping, /fix routing, and the copy-paste plugin contract.
  • Diff-only Gitleaks scan: no findings; public diff contains no deployment identities or private host data.
  • Remote proof: https://github.com/openclaw/openclaw/actions/runs/29672804482

AI-assisted. I understand the requester provenance, routing, and enforcement boundaries in this change.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation agents Agent runtime and tooling extensions: codex size: S maintainer Maintainer-authored PR labels Jul 19, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 1:28 PM ET / 17:28 UTC.

Summary
Adds optional host-derived requester facts to the public before_tool_call hook context and forwards them through managed coding tools and the Codex native hook relay, with plugin documentation and focused tests.

PR surface: Source +54, Tests +41, Docs +130. Total +225 across 11 files.

Reproducibility: yes. from source: start an active shared run as an owner, steer it with a lower-privileged allowed sender, then trigger a tool call; the review trace shows the hook context retains the original requester snapshot.

Review metrics: 1 noteworthy metric.

  • Public hook contract: 1 added requester field. A new plugin-facing authorization input needs explicit provenance and upgrade semantics before it becomes part of the supported SDK.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Fix requester freshness for steered or resumed active runs and add an adversarial regression test.
  • [P1] Add redacted real-run evidence showing hook requester facts for initial and steered messages.
  • Obtain maintainer confirmation of the public requester-contract semantics.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body reports tests, checks, a build, and a remote Actions run, but it does not provide inspectable after-fix real-run output proving requester facts reach a hook correctly—especially across steering. Add redacted live output, logs, or a terminal recording; redact private identities and endpoints. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] A lower-privileged sender can steer an active run started by an owner and inherit the owner-captured requester facts for subsequent hook decisions, defeating the documented fail-closed authorization policy.
  • [P1] Adding requester identity to the public plugin hook API commits OpenClaw to provenance, freshness, and compatibility semantics that need an explicit owner decision before merge.

Maintainer options:

  1. Refresh requester facts for steered messages (recommended)
    Carry the steered sender’s authenticated requester facts through the active-run queue and rebuild or refresh the hook context before each affected tool call.
  2. Reject cross-requester steering for protected runs
    If requester freshness cannot be preserved safely, prevent a different requester from steering an authorization-policy run.
  3. Pause the public authorization contract
    Do not merge the public requester field until maintainers choose and document its lifetime and authority semantics.

Next step before merge

  • [P1] A concrete correctness defect exists, but the permanent public authorization-contract semantics require maintainer direction before an automated repair should proceed.

Maintainer decision needed

  • Question: Should ctx.requester become a permanent public plugin authorization contract only after OpenClaw guarantees message-scoped freshness across steering and resumed active runs?
  • Rationale: The patch adds a new public plugin SDK field intended for authorization policy, so maintainers must choose its permanent provenance and lifecycle guarantee rather than accepting an implementation-specific snapshot.
  • Likely owner: pashpashpash — The decision centers on the native hook relay and plugin-hook boundary this contributor has previously worked on.
  • Options:
    • Define a fresh-per-message contract (recommended): Require each tool-hook call to receive requester facts from the message that caused that tool action, including steered messages, then retain the public API.
    • Restrict the initial contract: Keep requester context only for runs that cannot be steered across identities and defer broader authorization documentation until lifecycle coverage exists.
    • Defer the public API: Pause this public SDK addition and continue using existing channel admission and tool-policy controls until the requester lifecycle is designed end to end.

Security
Needs attention: The new authorization input can become stale across steered active runs, creating a concrete privilege-boundary regression.

Review findings

  • [P1] Refresh requester context for steered messages — src/agents/agent-tools.ts:1196
Review details

Best possible solution:

Make requester identity message-scoped for every tool invocation, or reject cross-requester steering for policy-protected runs; add adversarial coverage for owner-started runs steered by lower-privileged senders before documenting this as an authorization mechanism.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: start an active shared run as an owner, steer it with a lower-privileged allowed sender, then trigger a tool call; the review trace shows the hook context retains the original requester snapshot.

Is this the best way to solve the issue?

No. Forwarding requester facts only at tool-set construction is not safe for active runs that accept later steered messages; the hook context must be refreshed per causative message or cross-requester steering must be rejected.

Full review comments:

  • [P1] Refresh requester context for steered messages — src/agents/agent-tools.ts:1196
    requester is captured once while the tool set is created. An active shared run can later receive a steered prompt from another allowed sender without rebuilding these tools, so before_tool_call still sees the original owner’s sender ID, roles, and senderIsOwner; that lets the documented policy authorize owner-only tools for the steered sender. Carry authenticated requester facts with each steered message and refresh the hook context, or reject steering across requester identities.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6fb3aae404f6.

Label changes

Label changes:

  • add P1: The documented authorization use case can apply owner-level tool permission to a later lower-privileged sender in an active steered run.
  • add merge-risk: 🚨 security-boundary: A stale owner requester snapshot can authorize a tool call caused by a different, lower-privileged sender.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • remove P2: Current review triage priority is P1, so this older priority label is no longer current.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

  • P1: The documented authorization use case can apply owner-level tool permission to a later lower-privileged sender in an active steered run.
  • merge-risk: 🚨 compatibility: The PR adds a public plugin hook field whose required freshness and provenance semantics are not yet consistently enforced across the runtime lifecycle.
  • merge-risk: 🚨 security-boundary: A stale owner requester snapshot can authorize a tool call caused by a different, lower-privileged sender.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports tests, checks, a build, and a remote Actions run, but it does not provide inspectable after-fix real-run output proving requester facts reach a hook correctly—especially across steering. Add redacted live output, logs, or a terminal recording; redact private identities and endpoints. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +54, Tests +41, Docs +130. Total +225 across 11 files.

View PR surface stats
Area Files Added Removed Net
Source 6 55 1 +54
Tests 3 43 2 +41
Docs 2 130 0 +130
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 11 228 3 +225

Security concerns:

  • [high] Stale requester identity can authorize another sender — src/agents/agent-tools.ts:1196
    The requester identity used by plugin policy is stored at tool creation time even though a different allowed sender can steer the same active run later; a hook may therefore approve a tool call based on the original owner identity.
    Confidence: 0.96

What I checked:

  • Static requester capture: The changed tool-construction path builds one requester object from OpenClawCodingToolsOptions and places it in a reusable hook context, so subsequent tool calls reuse the original sender facts. (src/agents/agent-tools.ts:1196, ca6d6f6e99d2)
  • Steered-run escalation path: The recorded review identifies the existing steering path as injecting a later sender’s prompt into an active run without rebuilding tools or replacing requester identity; an owner-started run can therefore retain owner identity for a lower-privileged sender’s later tool call. (src/auto-reply/reply/agent-runner.ts:1412, ca6d6f6e99d2)
  • Missing per-message refresh boundary: The same review traces the active-run preparation boundary that does not pass later sender identity to the queue handle, leaving the PR’s create-time requester snapshot stale across steering. (src/agents/embedded-agent-runner/run/attempt-stream-prepare.ts:352, ca6d6f6e99d2)
  • Public SDK expansion: The branch adds requester to PluginHookToolContext, making sender identity a permanent plugin-facing authorization input rather than an internal implementation detail. (src/plugins/hook-types.ts:656, ca6d6f6e99d2)
  • Related prototype is not a replacement: The linked prototype explicitly says it is experimental and must not land; this PR is a narrower successor but does not supersede the need to repair the active-run steering invariant.

Likely related people:

  • pashpashpash: Repository release history credits this contributor for bridging Codex-native tool hooks into OpenClaw plugin hooks and approvals, the relay boundary extended by this PR. (role: native-hook relay contributor; confidence: medium; files: src/agents/harness/native-hook-relay.ts, extensions/codex/src/app-server/native-hook-relay.ts)
  • joshavant: Repository release history credits this contributor for recent Codex app-server and provider-routing work adjacent to the runtime path receiving requester context. (role: recent Codex/provider contributor; confidence: low; files: extensions/codex/src/app-server/run-attempt-resources.ts, extensions/codex/src/app-server/run-attempt.native-hook-relay.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-07-19T04:18:16.974Z sha f46f8cb :: needs real behavior proof before merge. :: none

@steipete steipete self-assigned this Jul 19, 2026
@steipete
steipete force-pushed the feat/file-authorization-hook branch from f46f8cb to 0324573 Compare July 19, 2026 17:16
@steipete
steipete marked this pull request as ready for review July 19, 2026 17:16

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0324573079

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/agents/agent-tools.ts
Comment on lines +1176 to +1181
const requester = {
...(turnSourceChannel ? { channel: turnSourceChannel } : {}),
...(options?.agentAccountId ? { accountId: options.agentAccountId } : {}),
...(options?.senderId ? { senderId: options.senderId } : {}),
...(options?.senderIsOwner !== undefined ? { senderIsOwner: options.senderIsOwner } : {}),
...(options?.memberRoleIds?.length ? { roleIds: [...options.memberRoleIds] } : {}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Refresh requester context for steered messages

When an owner starts a shared group run and a lower-privileged allowed sender posts while it remains active, the default steering path injects that sender's prompt into the existing run (src/auto-reply/reply/agent-runner.ts:1412) without rebuilding its tools or passing sender identity to the queue handle (src/agents/embedded-agent-runner/run/attempt-stream-prepare.ts:352). Because this requester object is captured only when the tool set is created, every subsequent before_tool_call still sees the original owner's senderIsOwner, sender ID, and roles, allowing the documented sender-aware policy to authorize owner-only tools prompted by the steered sender. Carry requester identity with each steered message and refresh the hook context, or reject steering across requester identities.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. P2 Normal backlog priority with limited blast radius. labels Jul 19, 2026
@steipete
steipete merged commit 4c55a4f into main Jul 19, 2026
136 of 142 checks passed
@steipete
steipete deleted the feat/file-authorization-hook branch July 19, 2026 17:32
@steipete

Copy link
Copy Markdown
Contributor Author

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 20, 2026
* feat(plugins): expose requester context to tool hooks

* chore(plugin-sdk): restore generated API baseline

* chore: drop release-owned changelog entry

* docs: refresh documentation map
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation extensions: codex maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant