Skip to content

docs(problems): add tool call risk assessment problem doc - #2009

Merged
rh-hemartin merged 5 commits into
fullsend-ai:mainfrom
Benkapner:docs/tool-call-risk-assessment
Jul 1, 2026
Merged

docs(problems): add tool call risk assessment problem doc#2009
rh-hemartin merged 5 commits into
fullsend-ai:mainfrom
Benkapner:docs/tool-call-risk-assessment

Conversation

@Benkapner

@Benkapner Benkapner commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • New problem document exploring pre-execution risk assessment for agent tool calls
  • Existing hooks (Tirith, SSRF validator, canary, unicode normalizer) catch known-bad patterns but miss context-dependent risks like force-pushes to protected branches, dynamic command construction via variable interpolation, or multi-step exfiltration sequences
  • Proposes four approaches with trade-offs: LLM-as-judge, behavioral baselines, declarative policies, and a hybrid
  • Includes open questions on latency, judge isolation, sequence analysis, and cold start

Complements #174 (Experiment: reasoning monitor agent for prompt injection detection) by addressing dangerous actions regardless of intent, rather than monitoring reasoning traces for compromise signals. Both layers are needed; neither subsumes the other.

Proposes approaches for evaluating the risk of individual tool calls
before execution, covering LLM-as-judge, behavioral baselines,
declarative policies, and hybrid approaches.

References issue fullsend-ai#174 (reasoning monitor).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [editorial-duplication] docs/problems/tool-call-risk-assessment.md:5 — The "Related:" block (lines 5–8) is duplicated verbatim at lines 10–12. This is a copy-paste error producing identical content twice.
    Remediation: Remove one of the two identical "Related:" blocks.

  • [architectural-coherence] docs/problems/tool-call-risk-assessment.md — The document proposes risk assessment mechanisms that would block tool calls for security reasons (Approach 1: "High: block, log" and "Critical: block, log, halt agent execution entirely"). This creates tension with ADR-0027's principle that "Security enforcement stays in the sandbox — the only layer the agent cannot bypass" and that "permissions.deny serves as a steering tool... Not a security control." Since this is a problem document exploring approaches (not a decision), explicitly acknowledging this tension would help readers understand how the proposals relate to the current architecture.
    Remediation: Add a note in the Approach 1 trade-offs or in the "Relationship to existing hooks" section acknowledging ADR-0027's sandbox-as-sole-enforcement-layer principle and clarifying whether these blocking mechanisms are intended as steering, escalation triggers, or a proposed expansion of the security boundary.

Low

  • [technical-accuracy] docs/problems/tool-call-risk-assessment.md:46 — The mitigation "use a different model family than the primary agent" cites Threat 4 of the security threat model for "model diversity as defense." The security threat model discusses model diversity in the context of detecting backdoors in agent-authored code (correlated blindspots between authoring and reviewing models), not in the context of hardening an LLM judge against poisoned-context manipulation. The underlying principle of model diversity is sound and transferable, but the specific citation does not directly support the threat scenario described.

  • [scope-coherence] docs/problems/tool-call-risk-assessment.md — The document's "Relationship to existing hooks" section explains the boundary well ("addresses the space between those specific checks, where the threat is context-dependent"). A brief note on why this is a standalone problem document rather than a section within security-threat-model.md would further clarify the scope.

  • [trajectory-alignment] docs/problems/tool-call-risk-assessment.md — The PR description mentions "Complements Experiment: reasoning monitor agent for prompt injection detection #174" without clarifying whether Experiment: reasoning monitor agent for prompt injection detection #174 authorizes this work or is merely related context. Since new problem documents are self-authorizing per AGENTS.md, this is a minor clarity improvement.

Previous run

Review

Findings

Medium

  • [architectural-clarity] docs/problems/tool-call-risk-assessment.md — The document proposes risk assessment mechanisms that would block tool calls for security reasons (Approach 1: "High: block, log" and "Critical: block, log, halt agent execution entirely"). This conflicts with the architectural principle established in ADR-0027: "Security enforcement stays in the sandbox — the only layer the agent cannot bypass" and "permissions.deny serves as a steering tool... Not a security control." The document should explicitly position its proposals relative to this principle — are these mechanisms intended as steering (like permissions.deny), as escalation triggers for human review, or as a proposed amendment to the sandbox-only security enforcement model?
    Remediation: Add a brief section acknowledging ADR-0027's sandbox-as-sole-enforcement-layer principle and clarifying whether these proposals extend, complement, or would require amending that principle.

Low

  • [technical-accuracy] docs/problems/tool-call-risk-assessment.md:46 — The mitigation "use a different model family than the primary agent" cites Threat 4 of the security threat model for model diversity as defense. The security threat model discusses model diversity in the context of code authoring and review (detecting backdoors invisible to one model family), not in the context of adversarial judge manipulation by poisoned context. The general principle of model diversity applies, but the specific citation does not directly support the threat scenario described.

  • [edge-case-correctness] docs/problems/tool-call-risk-assessment.md:39 — The action mapping specifies that high-risk calls are blocked but agent execution continues, with the agent seeing the block reason. This creates a retry-evasion vector: a compromised agent could use the block reason to reformulate the call to evade the judge. This gap is worth calling out in the Open Questions section.

Info

  • [technical-accuracy] docs/problems/tool-call-risk-assessment.md:12 — The document refers to the unicode hook as a "unicode normalizer." The actual hook (unicode_posttool.py) is a PostToolUse sanitizer whose primary security function is stripping invisible characters, though it does also perform NFKC normalization. Minor terminology nit.

  • [authorization-status] docs/problems/tool-call-risk-assessment.md — No linked issue authorizing this change. The contribution is self-authorizing per CLAUDE.md ("When adding new problem areas, create a new file in docs/problems/"). The topic aligns with existing security work (ADR-0027, issue Experiment: reasoning monitor agent for prompt injection detection #174).


Labels: PR adds a new problem document exploring tool call risk assessment, a security-adjacent topic.

Previous run

Review

Findings

Medium

  • [architectural-clarity] docs/problems/tool-call-risk-assessment.md — The document proposes risk assessment mechanisms that would block tool calls for security reasons (Approach 1: "High: block, log" and "Critical: block, log, halt agent execution entirely"). This conflicts with the architectural principle established in ADR-0027: "Security enforcement stays in the sandbox — the only layer the agent cannot bypass" and "permissions.deny serves as a steering tool... Not a security control." The document should explicitly position its proposals relative to this principle — are these mechanisms intended as steering (like permissions.deny), as escalation triggers for human review, or as a proposed amendment to the sandbox-only security enforcement model?
    Remediation: Add a brief section acknowledging ADR-0027's sandbox-as-sole-enforcement-layer principle and clarifying whether these proposals extend, complement, or would require amending that principle.

Low

  • [technical-accuracy] docs/problems/tool-call-risk-assessment.md:46 — The mitigation "use a different model family than the primary agent" cites Threat 4 of the security threat model for model diversity as defense. The security threat model discusses model diversity in the context of code authoring and review (detecting backdoors invisible to one model family), not in the context of adversarial judge manipulation by poisoned context. The general principle of model diversity applies, but the specific citation does not directly support the threat scenario described.

  • [edge-case-correctness] docs/problems/tool-call-risk-assessment.md:39 — The action mapping specifies that high-risk calls are blocked but agent execution continues, with the agent seeing the block reason. This creates a retry-evasion vector: a compromised agent could use the block reason to reformulate the call to evade the judge. This gap is worth calling out in the Open Questions section.

Info

  • [technical-accuracy] docs/problems/tool-call-risk-assessment.md:12 — The document refers to the unicode hook as a "unicode normalizer." The actual hook (unicode_posttool.py) is a PostToolUse sanitizer whose primary security function is stripping invisible characters, though it does also perform NFKC normalization. Minor terminology nit.

  • [authorization-status] docs/problems/tool-call-risk-assessment.md — No linked issue authorizing this change. The contribution is self-authorizing per CLAUDE.md ("When adding new problem areas, create a new file in docs/problems/"). The topic aligns with existing security work (ADR-0027, issue Experiment: reasoning monitor agent for prompt injection detection #174).

Previous run (2)

Review

Findings

Medium

  • [missing-doc] README.md — New problem document docs/problems/tool-call-risk-assessment.md is not linked from README.md. CLAUDE.md explicitly requires: "When adding new problem areas, create a new file in docs/problems/ and link it from README.md." All 22 existing problem documents are indexed in README.md; this one must be added for discoverability.
    Remediation: Add a bullet point entry in README.md's problem document list linking to docs/problems/tool-call-risk-assessment.md with a brief description, positioned thematically near the Security Threat Model entry.

  • [stale-reference] docs/problems/tool-call-risk-assessment.md:141 — The Open Questions section links to graduated-approval-policy.md but this file does not exist anywhere in the repository. Readers following this link will get a 404.
    Remediation: Either create the referenced document, remove the link, or replace with a general description (e.g., "a broader risk scoring system for approval routing") that doesn't imply the document exists.

Info

  • [technical-accuracy] docs/problems/tool-call-risk-assessment.md:12 — The document refers to the unicode hook as a "unicode normalizer." The actual hook (unicode_posttool.py) is a PostToolUse sanitizer that strips invisible characters. "Normalizer" implies canonicalization; the hook's primary function is detection and removal. Minor terminology nit — the functional description ("strips invisible characters") is accurate.

  • [authorization-status] docs/problems/tool-call-risk-assessment.md — No linked issue authorizing this change. However, README.md's contribution guide explicitly permits creating new problem docs without prior issue authorization, so this contribution type is self-authorizing. The topic aligns with existing security work (Threat 4 model diversity, ADR-0027 tool restrictions, issue Experiment: reasoning monitor agent for prompt injection detection #174 prompt injection detection).

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

- Can the judge model be meaningfully isolated from the same poisoned context that might compromise the primary agent?
- Should the risk assessment operate at the individual tool call level, or should it also consider sequences (e.g., "read credentials file" followed by "curl to external URL")?
- How do we handle the cold start problem for behavioral baselines in new repos or with new agent roles?
- What is the right model for the judge? It needs to be fast and cheap but capable enough to understand security context. Is a small fine-tuned model better than a general-purpose small model?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] stale-reference

The Open Questions section links to graduated-approval-policy.md but this file does not exist anywhere in the repository. Readers following this link will get a 404.

Suggested fix: Either create the referenced document, remove the link, or replace with a general description that doesn't imply the document exists.

Add README.md entry for the new problem document. Remove stale
cross-reference to graduated-approval-policy.md.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
@Benkapner

Copy link
Copy Markdown
Contributor Author

Addressed in b3fe09f. Response to each finding:

[missing-doc] Added a bullet point entry in README.md linking to docs/problems/tool-call-risk-assessment.md, positioned after the Security Threat Model entry.

[stale-reference] Removed the broken link to graduated-approval-policy.md and replaced with inline text that doesn't imply the document exists.

[technical-accuracy] The doc uses "unicode normalizer" which matches the function name in the codebase (UnicodeNormalizer in internal/security/unicode.go). The functional description ("strips invisible characters") is accurate as noted. No change made.

[authorization-status] Acknowledged, no action needed per the bot's own assessment.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 8, 2026
Benkapner added a commit to Benkapner/fullsend that referenced this pull request Jun 8, 2026
Correct autonomy-spectrum.md vs intent-representation.md distinction.
Replace broken tool-call-risk-assessment.md link with PR fullsend-ai#2009 reference.
Add README.md entry. Clarify Challenger as intra-agent verification,
not inter-agent disagreement. Add mixed-path PR semantics for CODEOWNERS
interaction. Qualify change-type scoring for additions of new attack
surface.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Benjamin Kapner <bkapner@redhat.com>

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Two non-blocking notes inline.

allow_domains:
- "github.com"
- "api.github.com"
Read:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[moderate] AGENTS.md asks problem docs to present options with trade-offs, not prescribe. "(recommended for exploration)" in the heading edges into prescription. Could drop it and let the trade-offs speak for themselves.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok ill remove "(recommended for exploration)" from the heading. the trade-offs section already makes the case for why a hybrid is worth exploring, so the heading doesn't really need to say it

- Unauthorized network access
- Scope violation (tool call unrelated to assigned task)

**Verdict structure:**

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[minor] If a compromised agent sees the block reason, it could use that feedback to reformulate the call and evade the judge on retry. Worth adding to Open Questions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

great point, i hadn't considered that.. i'll add it to open questions. it connects to the broader question of how much information a blocked tool call should return to the agent, there's a tension between giving the agent enough context to adjust legitimately and giving a compromised agent a roadmap for evasion.

@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 19, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 6:12 AM UTC · Ended 6:15 AM UTC
Commit: 4e21a60 · View workflow run →

Benkapner and others added 2 commits June 19, 2026 09:13
…essment

Remove prescriptive "(recommended for exploration)" from Approach 4
heading, and add evasion-via-block-reason concern to Open Questions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

Site preview

Preview: https://b4a3f948-site.fullsend-ai.workers.dev

Commit: a4fa684c9888accd309952cd5cb012206d6162c0

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 19, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 6:18 AM UTC · Ended 6:32 AM UTC
Commit: 4e21a60 · View workflow run →

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

Comment thread docs/problems/tool-call-risk-assessment.md
Comment thread docs/problems/tool-call-risk-assessment.md
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/docs User-facing documentation security Security threat model and related concerns type/idea Exploratory idea or new problem area and removed requires-manual-review Review requires human judgment labels Jun 19, 2026
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:18 AM UTC · Completed 6:32 AM UTC
Commit: 2e6ac34 · View workflow run →

@rh-hemartin

Copy link
Copy Markdown
Member

@ralphbean your input is needed again I think

@rh-hemartin

Copy link
Copy Markdown
Member

I think we can merge as ralph approved with non-blocking. Thanks for the contribution.

@rh-hemartin
rh-hemartin added this pull request to the merge queue Jul 1, 2026
Merged via the queue into fullsend-ai:main with commit db48f7a Jul 1, 2026
13 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 1, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 8:43 AM UTC · Completed 8:49 AM UTC
Commit: a4fa684 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2009 — Tool Call Risk Assessment Problem Doc

Timeline: Opened June 8 by Benkapner, merged July 1 by rh-hemartin. Docs-only PR adding a new problem exploration document and linking it from README.

What went well:

  • The review bot's first pass caught two real issues: missing README link (required by AGENTS.md) and a stale cross-reference to a nonexistent file. Both were promptly fixed by the author.
  • Human reviewer (ralphbean) provided high-quality non-blocking feedback that improved the document.
  • The author was responsive, addressing all feedback within a day of each review.
  • The bot correctly downgraded from CHANGES_REQUESTED to COMMENTED on re-review after blocking issues were resolved.

What the human caught that the bot missed:

  • ralphbean identified that "(recommended for exploration)" in a heading violated AGENTS.md's rule that problem docs should present options with trade-offs rather than prescribe solutions. This is a repo-specific content guideline that the review bot did not check. Filing a proposal for this gap.

Skipped proposals (covered by existing issues):

1 proposal filed.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/docs User-facing documentation requires-manual-review Review requires human judgment security Security threat model and related concerns type/idea Exploratory idea or new problem area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants