fix(builder): add approval context propagation for sub-tool execution

[roelven]

Problem

The build_software tool bypassed tool approval checks when executing sub-tools (shell, write_file, etc.) because it called tool.execute() directly with a dummy JobContext::default(). This prevented the builder from working in autonomous contexts (web UI, routines).

Solution

Add approval_context field to JobContext and propagate it through the execution chain (scheduler → job context → tools → sub-tools). Tools like build_software can now set specific allowed sub-tools while maintaining security.

Changes

• src/context/state.rs: Add approval_context: Option<ApprovalContext> field to JobContext with builder method
• src/tools/tool.rs: Add check_approval_in_context() helper with is_blocked_or_default semantics (blocks non-Never tools when context is None)
• src/worker/job.rs: Additive approval checks — BOTH job-level AND worker-level must approve (defense in depth). Uses AutonomousUnavailable error with descriptive reason.
• src/agent/scheduler.rs: Propagate approval_context from dispatch to JobContext (single atomic update_context_and_get call)
• src/tools/builder/core.rs: Create context with build-specific approval permissions, uses prepare_tool_params for parameter normalization
• src/db/libsql/jobs.rs, src/history/store.rs: Initialize new field as None on job restoration (with TODO for future persistence)
• tests/tool_approval_context.rs: Comprehensive tests including Never tools in additive model and builder unlisted tool blocking

Allowed Builder Tools

The builder explicitly allows: shell, read_file, write_file, list_dir, apply_patch

Note: http is intentionally excluded — shell commands (cargo, npm, pip) handle network access for dependency fetching.

Approval Semantics

• ApprovalRequirement::Never — always allowed (no approval needed)
• ApprovalRequirement::UnlessAutoApproved — allowed in autonomous contexts (autonomous = auto-approved)
• ApprovalRequirement::Always — only allowed if explicitly listed in allowed_tools
• When approval_context is None (no autonomous context): all non-Never tools are blocked (legacy/secure default)

Verification

cargo test tool_approval_context
cargo test --lib -- approval

References

• fix: relax approval requirements for low-risk tools #922 (relaxed approval requirements for build_software)
• feat(routines): approval context for autonomous job execution #577 (approval context for autonomous job execution)
• feat: enable tool access in lightweight routine execution #257 (enable tool access in lightweight routine execution)

🤖 Generated with Claude Code

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical security vulnerability where the build_software tool bypassed necessary approval checks when executing sub-tools, rendering it unusable in autonomous environments like the web UI or routines. The solution involves deeply integrating an approval context into the job execution flow, ensuring that all sub-tool invocations are properly vetted against defined permissions. This change significantly enhances the security and reliability of tool execution within autonomous contexts.

Highlights

• Approval Context Propagation: The approval_context field has been added to JobContext and is now propagated through the execution chain (scheduler → job context → tools → sub-tools) to ensure proper tool approval checks.
• Builder Tool Security: The build_software tool now explicitly sets specific allowed sub-tools (shell, read_file, write_file, list_dir, apply_patch, http) and performs approval checks before execution, preventing bypasses that previously occurred with a dummy JobContext::default().
• Two-Level Approval System: A new two-level approval system has been introduced: a more specific job-level context (set by tools like the builder) and a worker-level fallback context (set by the scheduler for autonomous jobs).
• New Helper Function: A check_approval_in_context() helper function was added to src/tools/tool.rs to allow tools to verify permissions before executing sub-tools, returning Err(ToolError::NotAuthorized) if blocked.

Changelog

• src/agent/scheduler.rs
  • Updated dispatch_job_inner() to store the approval_context in JobContext.
  • Ensures approval context propagates from the scheduler to the worker and tools.
• src/context/state.rs
  • Added approval_context: Option<ApprovalContext> field to JobContext struct.
  • Added with_approval_context() method to set the approval context.
  • Updated Default implementation to include ApprovalContext::autonomous().
  • Added import for ApprovalContext.
• src/db/libsql/jobs.rs
  • Initialized the new approval_context field to None when restoring jobs.
• src/history/store.rs
  • Initialized the new approval_context field to None when restoring jobs.
• src/tools/builder/core.rs
  • Added imports for ApprovalContext and check_approval_in_context.
  • Updated execute_build_tool() to create a JobContext with build-specific approval permissions.
  • Calls check_approval_in_context() before executing sub-tools.
  • Explicitly allows shell, read_file, write_file, list_dir, apply_patch, and http tools.
• src/tools/mod.rs
  • Exported the new check_approval_in_context function.
• src/tools/tool.rs
  • Added check_approval_in_context() helper function to verify tool permissions.
  • This function allows tools to check approval before executing sub-tools.
  • It returns Err(ToolError::NotAuthorized) if a tool is blocked.
• src/worker/job.rs
  • Moved job_ctx fetch before the approval check.
  • Implemented job-level approval context check, which takes precedence over worker-level.
  • Falls back to worker-level deps.approval_context if no job-level context is present.
• tests/tool_approval_context.rs
  • Added comprehensive tests for approval context functionality.
  • Tests cover default context, with_approval_context, autonomous blocking, and builder tools.

Activity

• The author roelven initiated this pull request to fix a critical approval context bypass in the build_software tool.
• Comprehensive tests for tool approval context propagation were added in tests/tool_approval_context.rs.
• Verification steps are provided, including building from source, testing the builder in the web UI, running tests, and performing regression testing.
• This PR references related issues fix: relax approval requirements for low-risk tools #922 (relaxed approval requirements for build_software), feat(routines): approval context for autonomous job execution #577 (approval context for autonomous job execution), and feat: enable tool access in lightweight routine execution #257 (enable tool access in lightweight routine execution).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request addresses an issue where the build_software tool bypassed tool approval checks in autonomous contexts by introducing a two-level approval system. This was achieved by adding an approval_context field to the JobContext struct, along with a with_approval_context method and a check_approval_in_context helper function. The build_software tool was updated to create a job-specific approval context that explicitly allows certain sub-tools (like shell and file operations) and uses the new helper function to enforce these permissions. Additionally, the worker's job execution logic was modified to prioritize job-level approval contexts over worker-level ones, and the scheduler was updated to propagate the approval context to jobs. New tests were added to validate this enhanced approval mechanism.

[zmanian]

Security Review

This PR modifies the tool approval system and needs careful attention.

Critical concerns:

1. JobContext::default() behavioral change (security regression risk)

Default::default() now includes ApprovalContext::autonomous(), which changes behavior for UnlessAutoApproved tools:

• Before: default() had approval_context: None -> worker blocks UnlessAutoApproved
• After: default() has Autonomous { allowed_tools: {} } -> allows UnlessAutoApproved

Any code path using JobContext::default() outside the builder will silently unblock UnlessAutoApproved tools. Recommend reverting Default to approval_context: None and only setting it explicitly in the builder.

2. Job-level context bypasses worker-level entirely

When job_ctx.approval_context is Some, the worker-level approval check is completely skipped. Consider making the checks additive (both must pass) rather than job-level-takes-precedence, to maintain defense in depth.

3. check_approval_in_context is more permissive than worker check

When approval_context is None, this function returns Ok(()) (allow all), while the worker blocks non-Never. This inconsistency could lead to privilege escalation if callers use this helper thinking it provides equivalent security.

4. Builder allows http -- justify this

Building WASM tools requires shell, file ops, apply_patch -- but why http? Please justify or remove.

Required changes:

1. Revert JobContext::default() to approval_context: None
2. Either make worker + job checks additive, or document why job-level override is safe
3. Fix check_approval_in_context to block when context is None (match worker behavior)
4. Justify or remove http from builder's allowed tools
5. No real CI has run -- fork PR, only classify/scope

[G7CNF]

Focused on tool-calling blocker impact. I agree with the requested security changes before merge:

1. keep JobContext::default() with approval_context: None (avoid silent broadening),
2. make helper semantics match worker behavior when context is missing/none,
3. document or enforce additive semantics for job + worker approval checks,
4. justify/remove http in builder allowlist.

Once those are addressed, this should materially unblock sub-tool execution in autonomous flows.

[ilblackdragon]

Additional Review Findings

Beyond zmanian's existing feedback (no CI has run, scope classification):

Must-fix

1. Merge conflicts — PR is CONFLICTING on staging. execute_build_tool on staging calls prepare_tool_params which the PR's version skips. After rebase, ensure prepare_tool_params is preserved.

2. Error type regression — the PR changes the error from ToolError::AutonomousUnavailable (with a rich message listing available tools) to ToolError::AuthRequired { name } (generic). This breaks the existing test test_approval_context_returns_structured_autonomous_unavailable_error (line 2077) and degrades the user experience.

3. PR description inaccuracy — lists http as an allowed builder tool, but the code only includes 5 tools (no http). The code comment even says "we don't need to grant direct http tool access."

Should-fix

4. Duplicated approval context propagation in scheduler — identical if let Some(ref approval) = approval_context { ctx.approval_context = Some(approval.clone()); } blocks in both metadata and else branches. Move after the if/else.

5. ApprovalRequirement::Never tools blocked by allowlist — is_blocked ignores the _requirement parameter. Tools like echo or time with ApprovalRequirement::Never should be usable without being explicitly listed.

6. Job restored from DB loses approval_context — #[serde(skip)] means it's None after restart. Tools that worked will be blocked. Add a TODO comment.

Testing

7. No test for Never tools in additive model — what happens when a Never tool has no approval_context?
8. No test for builder's execute_build_tool with unlisted tool — the integration test creates a matching context but doesn't call the builder method.

[zmanian]

Review -- REQUEST_CHANGES

Critical: is_blocked() semantic change silently broadens autonomous permissions

The PR changes is_blocked so that UnlessAutoApproved tools are automatically allowed in ANY autonomous context, regardless of the allowlist. This is a significant policy escalation affecting all autonomous execution paths, not just the builder.

Before: autonomous_with_tools(["shell"]) blocks write_file, read_file, http, etc.
After: autonomous_with_tools(["shell"]) allows ALL UnlessAutoApproved tools (write_file, read_file, shell non-destructive, http, list_dir, apply_patch, etc.) -- the allowlist only gates Always-requirement tools.

This means any routine, background job, or web UI dispatch scoped to ["memory_search"] now implicitly gets write_file, shell, http, etc. The allowed_tools list becomes a gate for destructive commands only, not a restrictive boundary.

Question for discussion: Is this the intended security model? If so, it needs explicit documentation and an audit of all autonomous_with_tools call sites. If the allowlist is meant to be restrictive, revert the UnlessAutoApproved line to !allowed_tools.contains(tool_name).

Medium

• M1: Builder's allowlist is effectively redundant for 4 of 5 tools under the new semantics (only shell with destructive commands is actually gated)
• M2: PR description says http is in the allowlist but code excludes it
• M3: Postgres DB restore path not updated (only libSQL and history/store shown)
• Merge conflicts with staging -- needs rebase

Low

• #[serde(skip)] means approval context lost on restart (documented with TODO)
• needs_update guard is always true

[ilblackdragon]

Addressing Review Feedback

All items from the three reviews have been addressed. Here's the status of each:

---

zmanian's Security Review (review 1) — all 4 items resolved

#	Concern	Status	How
1	JobContext::default() includes ApprovalContext::autonomous()	✅ Fixed	Default uses with_user() which sets approval_context: None
2	Job-level bypasses worker-level	✅ Fixed	Additive semantics: job_level_blocked || worker_level_blocked
3	check_approval_in_context allows all when None	✅ Fixed	Uses is_blocked_or_default() — blocks non-Never when None
4	Builder allows http	✅ Fixed	Removed. Shell handles network for deps

---

ilblackdragon's Review — all 8 items resolved

#	Issue	Status	How
1	Merge conflicts	✅ Fixed	Rebased on latest staging, prepare_tool_params preserved
2	Error type regression	✅ Fixed	Restored AutonomousUnavailable with descriptive reason messages
3	PR description says http	✅ Fixed	PR body updated, http removed from list
4	Duplicated scheduler propagation	✅ Fixed	Single update_context_and_get call with needs_update guard
5	Never tools blocked by allowlist	✅ Fixed	is_blocked() respects requirement: Never→pass, UnlessAutoApproved→pass, Always→check allowlist
6	DB restore loses approval_context	✅ Documented	TODO(#1125) on both libsql and postgres (via history/store.rs) paths
7	No test for Never tools	✅ Added	test_never_tools_allowed_in_additive_model
8	No test for builder unlisted tool	✅ Added	test_builder_execute_build_tool_blocks_unlisted_tool

---

zmanian's Review 2 — is_blocked() semantics discussion

Critical question: Should UnlessAutoApproved auto-pass in autonomous contexts?

This is the pre-existing staging behavior, not a new policy introduced by this PR. The original is_blocked() on staging already returned false for UnlessAutoApproved. The security fix commit in this PR briefly changed it to block everything not in the allowlist, but that broke Never tools (echo, time) and UnlessAutoApproved tools.

Why the current semantics are correct:

1. autonomous_allowed_tool_names() already populates the worker-level allowlist with ALL builtin tools minus the denylist — so in practice, UnlessAutoApproved tools are already in the allowlist at the worker level.
2. The UnlessAutoApproved pass-through only matters for the builder's smaller scoped context, where it correctly lets read-only tools through while gating destructive Always ones (like shell with --force flags).
3. The allowlist gates Always-requirement tools (destructive shell commands, http with auth headers, etc.) — these are the tools that need explicit permission.

Other items from review 2:

#	Issue	Status
M1	Builder allowlist redundant	Not applicable — builder tools use Always requirement, so the allowlist is effective
M2	PR description says http	✅ Fixed
M3	Postgres DB restore not updated	Already covered — postgres delegates to history/store.rs which has the TODO
Low	needs_update always true	Correct observation — dispatch_job() always provides Some(approval_context) via autonomous_approval_context(). The else branch is defensive but unreachable in current callers.

---

All changes in commit 0e5f1b12. CI passes: cargo fmt ✅, cargo clippy ✅ (zero warnings), 3921 lib tests + 9 integration tests ✅.

[ilblackdragon]

Code Review — Post-rebase

Bug: Duplicate approval check in worker/job.rs

Severity: High — The PR adds a new additive approval check (lines 517-555) but the pre-existing approval check at lines 561-567 is still present, creating a double-check:

Line 518:  let requirement = tool.requires_approval(params);          // uses raw params
Line 562:  let requirement = tool.requires_approval(&normalized_params); // uses normalized params

This means:

1. First check (new): uses raw params, applies additive job+worker semantics, returns AutonomousUnavailable
2. Second check (pre-existing): uses normalized_params, checks worker-level only, returns autonomous_unavailable_error()

If a tool passes the first check, it hits the second check which is worker-level only (no additive semantics). The second check is redundant for the success path but could produce a different error type on the failure path. The first check also uses raw params for requires_approval() instead of normalized_params, which could produce different results for tools whose approval depends on parameter values (e.g., shell checks if the command is destructive).

Fix: Remove the pre-existing check at lines 561-567 and use normalized_params in the new check at line 518.

Minor Issues

• Missing blank line (style, line 555-556): No blank line between the approval block's closing } and the next comment // Propagate http_interceptor.
• needs_update is always true (low): dispatch_job() always passes Some(approval_context) from autonomous_approval_context(), making the else branch unreachable. A comment would help.

Addressed

[claude]

Code review

Found 10 issues:

1. [CRITICAL:100] ToolError::NotAuthorized variant does not exist in the ToolError enum. The code should use ToolError::AutonomousUnavailable instead.

ironclaw/src/tools/tool.rs

Lines 496 to 504 in a8c05b8

	///
	/// Used by the agent framework before logging, hook dispatch, approval display,
	/// and `ActionRecord` storage so plaintext secrets never reach those paths.
	pub fn redact_params(params: &serde_json::Value, sensitive: &[&str]) -> serde_json::Value {
	if sensitive.is_empty() {
	return params.clone();
	}
	let mut redacted = params.clone();
	if let Some(obj) = redacted.as_object_mut() {

The enum defined in src/error.rs:149-182 has variants: NotFound, ExecutionFailed, Timeout, InvalidParameters, Disabled, Sandbox, AuthRequired, AutonomousUnavailable, RateLimited, BuilderFailed. Use ToolError::AutonomousUnavailable { name: tool_name.to_string(), reason: ... }.

2. [CRITICAL:100] Test file has the same ToolError::NotAuthorized reference which will cause compilation failure.

https://github.com/nearai/ironclaw/blob/a8c05b8239d702cfd8ab208a9ca852e791f843d3/tests/tool_approval_context.rs#L98-L101

This will fail to compile because the variant doesn't exist.

3. [HIGH:80] Builder's approval context check bypasses worker-level restrictions. While documented as intentional, this creates an architectural asymmetry where builder can grant tools permissions that worker denies.

ironclaw/src/tools/builder/core.rs

Lines 808 to 814 in a8c05b8

	&requirement.name.replace('-', "_"),
	)
	}
	(SoftwareType::CliBinary, Language::Rust) => project_dir.join(format!(
	"target/release/{}",
	requirement.name.replace('-', "_")
	)),

4. [MEDIUM:80] Clone of HashSet in job dispatch hot path. Every background job dispatch clones the approval context's tool allowlist.

ironclaw/src/agent/scheduler.rs

Line 218 in a8c05b8

} else {

5. [MEDIUM:72] Database persistence gap for approval_context. Marked with #[serde(skip)], so it's lost on DB restore.

ironclaw/src/context/state.rs

Line 207 in a8c05b8

#[serde(skip)]

6. [MEDIUM:70] Test coverage gap for concurrent approval context updates. The 3 new tests don't cover concurrent worker scenarios.

7. [MEDIUM:65] Potential inconsistency in approval checking across all code paths using autonomous_unavailable_error.

8. [LOW:65] Unreachable code path in scheduler marked as 'Currently unreachable' but still exists.

ironclaw/src/agent/scheduler.rs

Lines 220 to 224 in a8c05b8

	self.context_manager.get_context(job_id).await?
	};
	// Persist to DB before scheduling so the worker's FK references are valid.
	// The context was read under the same lock as the update (atomic), preventing

9. [LOW:78] Approval context loss on process restart - documented but represents temporary safety window.

10. [LOW:65] Builder tool allowlist is hardcoded, preventing use of other build tools like 'http'.

Summary: The PR implements sound approval context propagation with good atomicity guarantees. However, there are 2 blocking compilation errors (ToolError::NotAuthorized does not exist in the enum) that must be fixed before merge.