Repository navigation
feat(docs): HUMAN IN THE LOOP - User consent for some tools execution - #109
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdded a new documentation file describing a Human-in-the-Loop pattern for secure AI tool execution, including definitions, two-step permission flow, components, technical workflow, TypeScript quick-start snippets, diagrams, security features, risk criteria, best practices, FAQs, and getting-started steps. No source code or public APIs were changed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
docs/HUMAN-IN-THE-LOOP.md (8)
159-165: Enrich tool metadata with identity and risk rationale.Add a stable toolId and rationale to help registries, policy engines, and audits.
Apply this diff:
-// Mark a tool as requiring confirmation -const deleteFileTool = { - name: "deleteFile", - description: "Deletes a file from the filesystem", - requiresConfirmation: true, // ← This makes it safe! -}; +// Mark a tool as requiring confirmation (with identity and rationale) +const deleteFileTool = { + toolId: "file.delete", // stable identifier + name: "deleteFile", + description: "Deletes a file from the filesystem", + requiresConfirmation: true, // policy gate + riskRationale: [ + "Irreversible deletion", + "Potential data loss", + ], +};
65-77: Use consistent naming (camelCase) and avoid global, mutable flags in diagrams.Mixing confirmation_received (snake_case) with requiresConfirmation (camelCase) is inconsistent, and the notion of a global flag is risky. Prefer confirmationReceived or, better, a “grant” concept.
Apply this diff for naming consistency in the sequence diagram:
- Note right of Tool Executor: Tool requires confirmation<br/>and confirmation_received is false. + Note right of Tool Executor: Tool requires confirmation<br/>and confirmationReceived is false. @@ - Note right of Tool Executor: confirmation_received is now true.<br>Reset it to false and execute the tool. + Note right of Tool Executor: A valid one-time grant is present.<br>Consume it atomically and execute the tool.Follow-up: elsewhere replace confirmation_received with a one-time grant model.
134-139: Replace confirmation_received checks with “has valid one-time grant?” and adjust UI step.Aligns terminology and security model in the flowchart.
Apply this diff:
- C -- Yes --> E{"Is confirmation_received?"}; + C -- Yes --> E{"Valid one-time grant for this request?"}; - E -- No --> F["Return 'Confirmation Required' to LLM"]; + E -- No --> F["Return USER_CONFIRMATION_REQUIRED"]; @@ - H -- Yes --> I["UI sets confirmation_received = true"]; + H -- Yes --> I["UI requests/receives a signed grant from backend"];
93-98: Tighten risk criteria wording and include network/internal access.“Change databases” is vague; add internal network access as risky.
Apply this diff:
-- **Change databases** (data modifications) +- **Modify databases** (writes, schema changes, bulk updates) @@ - **Access sensitive data** (privacy concerns) +- **Access internal networks/services** (lateral movement, data exfiltration)
151-153: Clarify audit logging to avoid sensitive-data leakage.Recommend logging digests/summaries rather than raw params and define retention.
Apply this diff:
-- **Clear audit trail** - Everything is logged for accountability +- **Privacy-respecting audit trail** - Log toolId, requestId, params digests (not raw data), userId, timestamps; define retention and access controls
193-207: Add developer best practices: idempotency, dry-run/preview, RBAC, rate limits, and server-side enforcement.These guardrails materially improve safety in production.
Apply this diff:
- **Test the flow** - Ensure confirmations work smoothly in your UI - **Log everything** - Keep records of what users approved +- **Prefer dry-run/preview** - Provide diffs or impact summaries before execution +- **Enforce server-side** - Executor must validate/consume grants; never trust client or LLM text +- **Ensure idempotency** - Use idempotency keys to prevent duplicate side effects on retries +- **Apply RBAC/ABAC** - Restrict who can approve which tools/scopes +- **Rate limit & budget guardrails** - Cap frequency/cost for risky tools +- **Handle concurrency** - Support multiple pending confirmations keyed by requestId
15-18: Minor wording tighten-up.Eliminate “by mistake” duplication and use more direct phrasing.
Apply this diff:
-- Modify important data by mistake +- Accidentally modify important data
224-231: Optional: add a row about auditability and replay protection.Helps readers understand the token/consumption benefits.
Apply this diff:
| ❌ No audit trail | ✅ Clear accountability | +| ❌ Flags can be replayed | ✅ One-time, scope-bound grants |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/HUMAN-IN-THE-LOOP.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/HUMAN-IN-THE-LOOP.md
[style] ~16-~16: ‘by mistake’ might be wordy. Consider a shorter alternative.
Context: ...the wrong files - Modify important data by mistake - Trigger costly operations you didn't ...
(EN_WORDINESS_PREMIUM_BY_MISTAKE)
[grammar] ~24-~24: There might be a mistake here.
Context: ...ou time to review what's about to happen - ✅ Putting you in control - You decid...
(QB_NEW_EN)
[style] ~38-~38: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ou first 2. It explains exactly what it wants to do 3. It waits for your response ### S...
(REP_WANT_TO_VB)
[grammar] ~93-~93: There might be a mistake here.
Context: ...lete or modify files** (file operations) - Change databases (data modifications) ...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...Change databases* (data modifications) - Cost money (paid API calls) - **Can't ...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...tions) - Cost money (paid API calls) - Can't be undone (irreversible actions)...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...Can't be undone** (irreversible actions) - Access sensitive data (privacy concern...
(QB_NEW_EN)
[grammar] ~213-~213: There might be a mistake here.
Context: ...n off confirmations for tools I trust?** A: Yes, developers can adjust which tool...
(QB_NEW_EN)
[grammar] ~216-~216: There might be a mistake here.
Context: ...happens if I accidentally click "yes"?** A: The permission is only used once, so ...
(QB_NEW_EN)
[grammar] ~219-~219: There might be a mistake here.
Context: ...Q: Can the AI remember my preferences?** A: Not automatically - each risky action...
(QB_NEW_EN)
[grammar] ~224-~224: There might be a mistake here.
Context: ... | With HITL | | ---------------------------- | -------...
(QB_NEW_EN)
[grammar] ~225-~225: There might be a mistake here.
Context: ...-------- | --------------------------- | | ❌ AI acts immediately | ✅ AI ask...
(QB_NEW_EN)
[grammar] ~226-~226: There might be a mistake here.
Context: ...ely | ✅ AI asks permission first | | ❌ Mistakes can be costly | ✅ Mistak...
(QB_NEW_EN)
[grammar] ~227-~227: There might be a mistake here.
Context: ...costly | ✅ Mistakes are prevented | | ❌ User feels out of control | ✅ User s...
(QB_NEW_EN)
[grammar] ~228-~228: There might be a mistake here.
Context: ...f control | ✅ User stays in control | | ❌ Trust issues | ✅ Trust ...
(QB_NEW_EN)
[grammar] ~229-~229: There might be a mistake here.
Context: ... | ✅ Trust is built | | ❌ No audit trail | ✅ Clear ...
(QB_NEW_EN)
[grammar] ~234-~234: There might be a mistake here.
Context: ...s** - What actions could cause problems? 2. Implement confirmation flow - Add the ...
(QB_NEW_EN)
| LLM->>Application UI: 3. "I need to delete /tmp/log.txt. Is it okay?" | ||
|
|
||
| alt User Confirms |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not trust LLM text for permission gating; the executor/UI must enforce policy.
Clarify that the UI prompt is triggered by the executor’s policy decision and metadata, not because “the LLM asked.” This mitigates prompt-injection and social engineering.
Apply this diff:
- LLM->>Application UI: 3. "I need to delete /tmp/log.txt. Is it okay?"
+ LLM->>Application UI: 3. Request indicates deletion of /tmp/log.txt
+ Note over Application UI: The confirmation prompt is driven by the executor's policy & metadata, not by LLM text.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LLM->>Application UI: 3. "I need to delete /tmp/log.txt. Is it okay?" | |
| alt User Confirms | |
| LLM->>Application UI: 3. Request indicates deletion of /tmp/log.txt | |
| Note over Application UI: The confirmation prompt is driven by the executor's policy & metadata, not by LLM text. | |
| alt User Confirms |
| 1. **Confirmation Flag** - Tools are marked as `requiresConfirmation: true` | ||
| 2. **Smart Registry** - System tracks which tools need approval | ||
| 3. **Execution Wrapper** - Intercepts risky tool calls | ||
| 4. **Permission State** - Remembers your "yes" for exactly one operation | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reframe “Permission State” as a one-time permission grant (token) with scope and TTL.
A mutable “state/flag” invites replay and cross-request reuse. Define it as a verifiable, consumable grant bound to the exact action.
Apply this diff:
-4. **Permission State** - Remembers your "yes" for exactly one operation
+4. **One-time Permission Grant (token)** - A verifiable, single-use grant bound to {requestId, toolId, paramsDigest} with a short TTL; validated and consumed atomically by the executor📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. **Confirmation Flag** - Tools are marked as `requiresConfirmation: true` | |
| 2. **Smart Registry** - System tracks which tools need approval | |
| 3. **Execution Wrapper** - Intercepts risky tool calls | |
| 4. **Permission State** - Remembers your "yes" for exactly one operation | |
| 1. **Confirmation Flag** - Tools are marked as `requiresConfirmation: true` | |
| 2. **Smart Registry** - System tracks which tools need approval | |
| 3. **Execution Wrapper** - Intercepts risky tool calls | |
| 4. **One-time Permission Grant (token)** - A verifiable, single-use grant bound to {requestId, toolId, paramsDigest} with a short TTL; validated and consumed atomically by the executor |
🤖 Prompt for AI Agents
In docs/HUMAN-IN-THE-LOOP.md around lines 103 to 107, replace the mutable
"Permission State" bullet with a description that it's a one-time permission
grant (token) with explicit scope and TTL: explain that the grant must be
cryptographically verifiable and consumable, bound to the exact action
(action-id and user-id), include an expiration (TTL) and allowed scope, and must
be marked consumed on first use to prevent replay; describe acceptable
implementation patterns (signed short-lived token or server-side single-use
entry keyed by nonce) and note that mutable flags must be replaced by verifiable
single-use tokens that are invalidated after consumption.
| flowchart TD | ||
| subgraph "Execution: The Confirmation Checkpoint" | ||
| A["LLM requests to execute a tool"] --> B["Call executeTool"]; | ||
| B --> C{"Is tool in confirmation list?"}; | ||
| C -- No --> D["Execute tool directly"]; | ||
| C -- Yes --> E{"Is confirmation_received?"}; | ||
| E -- No --> F["Return 'Confirmation Required' to LLM"]; | ||
| E -- Yes --> G["Reset flag & Execute tool"]; | ||
| F --> H{"User is prompted"}; | ||
| H -- Yes --> I["UI sets confirmation_received = true"]; | ||
| H -- No --> L["Terminate & Inform LLM"]; | ||
| I --> B; | ||
| D --> K["Return final result"]; | ||
| G --> K; | ||
| L --> K; | ||
| end | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update flowchart to validate a scoped grant instead of a global confirmation flag.
The decision point should verify a valid, single-use grant bound to requestId/tool/params, not a shared flag. This eliminates TOCTOU and concurrent-call hazards.
Apply this diff in the Mermaid block:
- C -- Yes --> E{"Is confirmation_received?"};
- E -- No --> F["Return 'Confirmation Required' to LLM"];
- E -- Yes --> G["Reset flag & Execute tool"];
+ C -- Yes --> E{"Valid one-time grant for {requestId, toolId, paramsDigest}?"};
+ E -- No --> F["Return USER_CONFIRMATION_REQUIRED with {requestId, toolId, paramsDigest}"];
+ E -- Yes --> G["Consume grant atomically & Execute tool"];
- H -- Yes --> I["UI sets confirmation_received = true"];
- H -- No --> L["Terminate & Inform LLM"];
+ H -- Yes --> I["UI requests a signed one-time grant from backend"];
+ H -- No --> L["Terminate & Inform LLM"];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| flowchart TD | |
| subgraph "Execution: The Confirmation Checkpoint" | |
| A["LLM requests to execute a tool"] --> B["Call executeTool"]; | |
| B --> C{"Is tool in confirmation list?"}; | |
| C -- No --> D["Execute tool directly"]; | |
| C -- Yes --> E{"Is confirmation_received?"}; | |
| E -- No --> F["Return 'Confirmation Required' to LLM"]; | |
| E -- Yes --> G["Reset flag & Execute tool"]; | |
| F --> H{"User is prompted"}; | |
| H -- Yes --> I["UI sets confirmation_received = true"]; | |
| H -- No --> L["Terminate & Inform LLM"]; | |
| I --> B; | |
| D --> K["Return final result"]; | |
| G --> K; | |
| L --> K; | |
| end | |
| ``` | |
| flowchart TD | |
| subgraph "Execution: The Confirmation Checkpoint" | |
| A["LLM requests to execute a tool"] --> B["Call executeTool"]; | |
| B --> C{"Is tool in confirmation list?"}; | |
| C -- No --> D["Execute tool directly"]; | |
| C -- Yes --> E{"Valid one-time grant for {requestId, toolId, paramsDigest}?"}; | |
| E -- No --> F["Return USER_CONFIRMATION_REQUIRED with {requestId, toolId, paramsDigest}"]; | |
| E -- Yes --> G["Consume grant atomically & Execute tool"]; | |
| F --> H{"User is prompted"}; | |
| H -- Yes --> I["UI requests a signed one-time grant from backend"]; | |
| H -- No --> L["Terminate & Inform LLM"]; | |
| I --> B; | |
| D --> K["Return final result"]; | |
| G --> K; | |
| L --> K; | |
| end |
🤖 Prompt for AI Agents
In docs/HUMAN-IN-THE-LOOP.md around lines 129 to 145, the flowchart currently
checks a shared confirmation_received flag; replace that decision with a
validation of a single-use, scoped grant bound to requestId/tool/params (e.g.,
"Is valid grant for requestId/tool/params?"), branch No to return "Confirmation
Required" or reject, branch Yes to invalidate the grant and execute the tool,
and add an explicit transition for grant invalidation/reset after use to prevent
TOCTOU and concurrent-call hazards while keeping the rest of the flow intact.
| // When AI requests risky action | ||
| if (toolRequiresConfirmation && !userHasConfirmed) { | ||
| // Show user a confirmation dialog | ||
| showConfirmationDialog({ | ||
| action: "Delete file", | ||
| details: "This will permanently delete 'important.txt'", | ||
| onConfirm: () => executeToolSafely(), | ||
| onCancel: () => tellAIUserSaidNo(), | ||
| }); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bind UI confirmation to a specific request and include a preview/impact summary.
Tie the prompt to requestId and a params digest; capture cost/time estimates and show a diff/preview if available. The UI should request a signed grant from backend rather than toggling state locally.
Apply this diff:
-// When AI requests risky action
-if (toolRequiresConfirmation && !userHasConfirmed) {
- // Show user a confirmation dialog
- showConfirmationDialog({
- action: "Delete file",
- details: "This will permanently delete 'important.txt'",
- onConfirm: () => executeToolSafely(),
- onCancel: () => tellAIUserSaidNo(),
- });
-}
+// When AI requests risky action
+if (toolRequiresConfirmation && !hasValidGrant(requestId)) {
+ // Show user a confirmation dialog (scoped to this requestId)
+ showConfirmationDialog({
+ action: "Delete file",
+ details: "This will permanently delete 'important.txt'",
+ paramsDigest, // display canonicalized params summary
+ costEstimate, // optional: estimated cost/time/affected records
+ preview: changePreview, // optional: dry-run/diff if tool supports it
+ onConfirm: async () => {
+ const grant = await requestPermissionGrant({ requestId, toolId, paramsDigest });
+ await executeToolSafely(grant);
+ },
+ onCancel: () => tellAIUserSaidNo({ requestId }),
+ });
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // When AI requests risky action | |
| if (toolRequiresConfirmation && !userHasConfirmed) { | |
| // Show user a confirmation dialog | |
| showConfirmationDialog({ | |
| action: "Delete file", | |
| details: "This will permanently delete 'important.txt'", | |
| onConfirm: () => executeToolSafely(), | |
| onCancel: () => tellAIUserSaidNo(), | |
| }); | |
| } | |
| ``` | |
| // When AI requests risky action | |
| if (toolRequiresConfirmation && !hasValidGrant(requestId)) { | |
| // Show user a confirmation dialog (scoped to this requestId) | |
| showConfirmationDialog({ | |
| action: "Delete file", | |
| details: "This will permanently delete 'important.txt'", | |
| paramsDigest, // display canonicalized params summary | |
| costEstimate, // optional: estimated cost/time/affected records | |
| preview: changePreview, // optional: dry-run/diff if tool supports it | |
| onConfirm: async () => { | |
| const grant = await requestPermissionGrant({ requestId, toolId, paramsDigest }); | |
| await executeToolSafely(grant); | |
| }, | |
| onCancel: () => tellAIUserSaidNo({ requestId }), | |
| }); | |
| } |
| // Only execute after user confirms | ||
| function executeToolSafely() { | ||
| setUserConfirmation(true); // Grant permission | ||
| executeTool(); // Run the tool | ||
| setUserConfirmation(false); // Reset permission immediately | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace boolean toggle with one-time, scope-bound permission grant to avoid race/replay.
The boolean setUserConfirmation(true/false) is unsafe in concurrent/async contexts. It can be reused by another in-flight tool call (race), replayed later, or flipped by compromised UI. Use a one-time permission token bound to requestId + toolId + params digest with a short TTL, and consume it atomically on the server-side executor.
Apply this diff to the snippet:
-// Only execute after user confirms
-function executeToolSafely() {
- setUserConfirmation(true); // Grant permission
- executeTool(); // Run the tool
- setUserConfirmation(false); // Reset permission immediately
-}
+// Only execute after user confirms — use a one-time, scope-bound grant
+async function executeToolSafely(grant: { requestId: string; token: string }) {
+ // The executor must validate and CONSUME the grant atomically server-side.
+ await executeTool({ requestId: grant.requestId, permissionToken: grant.token });
+}Support code to add outside this snippet (for clarity):
// Executor-side (authoritative):
// - Verifies token signature
// - Ensures token matches {requestId, toolId, paramsDigest}
// - Checks TTL and nonces
// - Consumes/invalidates the token atomically before executing
type PermissionGrant = {
requestId: string;
toolId: string;
paramsDigest: string; // e.g., SHA-256 of canonicalized params
exp: number; // unix epoch ms
jti: string; // unique nonce (prevent replay)
sig: string; // signature (HMAC/JWT/etc.)
};🤖 Prompt for AI Agents
In docs/HUMAN-IN-THE-LOOP.md around lines 185 to 191, replace the unsafe boolean
toggle pattern with a one-time, scope-bound permission token: stop calling
setUserConfirmation(true/false); instead generate/obtain a PermissionGrant token
tied to {requestId, toolId, paramsDigest} with exp and jti, pass that token into
the executor call, and have the executor verify signature, TTL,
request/tool/params match and atomically consume/invalidate the token before
executing the tool; update the snippet to accept and forward the token rather
than toggling shared state and remove any immediate boolean resets.
85ff454 to
ed549c7
Compare
2 participants
Pull Request
Description
Type of Change
Related Issues
Changes Made
AI Provider Impact
Component Impact
Testing
Test Environment
Performance Impact
Breaking Changes
Screenshots/Demo
Checklist
Additional Notes
Summary by CodeRabbit