Repository navigation
fix(security_gate): give a refused approval a reason the model can read - #6810
Conversation
park_for_approval turned GateOutcome::Deny into Prompted { approved: false },
which carries no reason, so the harness told the model only "tool call was
not approved". After a Deny the model asked the user to approve again, or
reached the same result through shell.
The GateOutcome -> GateDecision mapping moves into decision_for_outcome. A
refusal is now a Deny whose text names no human and no timeout and rules out
other routes. Approvals and the audit id are unchanged; the middleware path
and the gate's own reason text are untouched.
Closes tinyhumansai#6808
Tiny Sweeper review
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughApproval refusals and timeouts now return a ChangesApproval gate outcomes
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The refusal and timeout flows deliver the intended guidance to the model, and approved outcomes remain allowed prompts. No merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Refused actions now receive explicit instructions against retrying or using another route. Approval and audit behavior remain intact. No new authorization weakness was established, but downstream handling was not fully verified. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
A rabbit checks the gate at night Comment |
gate_decision allows every command class while [autonomy] enabled = false, the default; it is not tied to the full tier.
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/openhuman-core/src/agent/tinyagents/host/security_gate.rs, crates/openhuman-core/src/agent/tinyagents/host/security_gate_tests.rs.
$0.0050 · 42,279 in / 11,409 out · 12,544 cached (30%) · ladder/vectors, deepseek/deepseek-v4-flash · 479 embedded
tests: $0.0011 · 13,625 in / 3,252 out · 9,728 cached (71%) · deepseek/deepseek-v4-flash
description: $0.0011 · 6,572 in / 3,743 out · 2,304 cached (35%) · deepseek/deepseek-v4-flash
e2e: $0.0020 · 18,116 in / 1,884 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tinysweeper
Bot
added
the
priority: p2
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…lass The [policy-denied] marker classifies as a policy failure with a zero recovery budget in repeated_failure, which pauses the turn before the model can answer. approval_gate_deny_flow and approval_gate_timeout caught it: the turn ended with the no-progress summary instead of the model's reply. The unmarked text classifies as Unknown, as the harness fallback did, so the model gets the reason and still answers this turn.
…ason approval_gate_deny_flow and approval_gate_timeout checked only the turn's final text, which a scripted model produces whatever it was sent. They now also assert the follow-up request carries the gate's refusal and not the reason-less "tool call was not approved" fallback.
Summary
SecurityGatepath, the model now gets aDenywith a reason. Before, it gotPrompted { approved: false }, which carries none.park_for_approval. A refused approval now returnsDeny; an approval still returnsPrompted { approved: true }.Problem
park_for_approvalloggedGateOutcome::Deny { reason }and returnedPrompted { approved: false }. The harness (tinyagents-harness/src/agent_loop/tools.rs~905) readsdenial_reason(), which isNoneforPrompted, and falls back to"tool call was not approved".shell: gh gist create --publicand published anyway.shellnever prompts while[autonomy] enabled = false(the default).ApprovalSecurityMiddlewarepath already passes the gate's reason to the model. Only this adapter dropped it.Solution
GateOutcome→GateDecisionmapping moves into a pure fn,decision_for_outcome.park_for_approvalstill writes the audit id onAllow, then calls it.Allow→Prompted { approved: true }, as before.Deny { .. }→GateDecision::deny("This action was refused and must not be performed this turn — do not retry this call and do not achieve the same result another way (shell, CLI, another tool). Tell the user it was not done.").[policy-denied]marker, on purpose.middleware::repeated_failureclassifies that marker as a policy failure with a zero recovery budget. That pauses the turn before the model can answer, and the user sees "I stopped this turn early…" instead of an acknowledgement. The first push carried the marker, and CI'sapproval_gate_deny_flowandapproval_gate_timeoute2e tests caught exactly this. The unmarked text classifies asUnknown, the same as the old fallback, so the model gets the reason and still answers. The unit test pins that classification.GateOutcome::Deny, so they get the same neutral text. Passing the gate's own text through would say "User denied" or "timed out", which breaks the no-human-hint intent of the original design. Before this change, those cases got the bare fallback string.gate_intercept_decision.rs) and the middleware path are unchanged. So istests/raw_coverage/.../tool_registry_approval_raw_coverage_e2e.rs:1603, which still asserts "User denied" on the gate's text.Deny { reason }as host-authored text that is safe to show a model, so no vendor change is needed.Submission Checklist
a_refused_approval_is_a_deny_the_model_can_readcovers the human-deny and timeout reasons;an_approved_park_is_still_prompted_and_allowedcovers approval. Revert-check: withGateDecision::Prompted { approved: false }put back in theDenyarm ofdecision_for_outcome,a_refused_approval_is_a_deny_the_model_can_readfails atsecurity_gate_tests.rs:475, the.expect("a refused approval carries a reason for the model")ondenial_reason(). The approve test still passes. E2E:agent_harness_e2e::approval_gate_deny_flowandapproval_gate_timeoutnow also assert that the model's follow-up request carries the refusal text and not the"tool call was not approved"fallback (assert_model_saw_the_refusal). With the same revert, both fail atagent_harness_e2e.rs:184("the model must be told the refused call may not be done another way"); approve and installed still pass.park_for_approvalneeds the process-globalApprovalGate, which unit tests must not install, so it was already uncovered.Closes #6808below.Impact
ApprovalGate: after a refused, timed-out or cancelled approval, the model gets an explicit do-not-do-it-another-way instruction instead of "tool call was not approved".shell. It is prompt text, not enforcement.static:module-pins,rust-fmt,rust-layout,agent-runtime-boundaryandignored-tests-ratchetall fail, and each names only files this PR does not touch. Five nextest failures do not involve the approval path:orchestrator_advertises_direct_mcp_tools,orchestrator_cannot_install_a_skill_through_the_raw_registry_tool,orchestrator_hands_skill_installs_to_skill_setup_directly,cwd_jail_e2e::default_backend_is_named_and_availableandcredential_scrub::browser_confirmation_token_survives_without_exempting_page_credentials. I believe they come from chore(vendor): repoint all vendor modules to their merged upstream main #6809's bumps, but no main run exists to prove it.Related
ApprovalSecurityMiddlewarepath, whose refusal text keeps the marker; deliberately not fixed here)AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/deny-reason-reaches-modelValidation Run
pnpm --filter openhuman-app format:check, no frontend changepnpm typecheck, no frontend changeRUST_MIN_STACK=67108864 cargo test -p openhuman --lib -- agent::tinyagents::host::security_gate security::approval agent::tinyagents::policy_denial→ 161 passed, 0 failed;cargo test -p openhuman-cli --test agent_harness_e2e -- approval_gate→ 4 passed (deny, timeout, approve, installed), including the new request assertionrustfmt --checkon both touched files is clean;cargo clippy -p openhuman --lib -- -D warningsis cleanValidation Blocked
command:cargo clippy -p openhuman --lib --tests -- -D warningsandcargo fmt --checkerror:both fail on files this PR does not touch (e.g.agent/goals/store.rsimport order). The only--testsclippy hits in a touched file are atsecurity_gate_tests.rs:342/368, which predate this diff.impact:none from this PRBehavior Changes
GateDecision::Denywith neutral, route-closing text instead ofPrompted { approved: false }.Parity Contract
ApprovalSecurityMiddleware, and the gate's own reason strings.Promptedisauthorize_tool's channel stage, which matchesapproved: trueand returns any other decision as-is.Duplicate / Superseded PR Handling
Summary by CodeRabbit