security(use_ros): blocklist for safety-critical publish topics

[CurrentlyAWey]

Summary

Add a default blocklist preventing LLM-initiated publish to safety-critical ROS 2 topics via use_ros, gated by a human-in-the-loop (HIL) interrupt.

• Blocked topics: /cmd_vel, /cmd_vel_unstamped, /joint_command, /joint_trajectory, /joint_trajectory_controller/joint_trajectory, /emergency_stop, /e_stop, /motor_enable, /enable_motor, /disable_motor, /navigate_to_pose, /follow_path
• Blocklist matches both exact and namespace-stripped forms (e.g., /my_robot/cmd_vel matches /cmd_vel)

Three operating modes

Mode	Mechanism	When
Interactive	tool_context.interrupt() prompts operator	Default — agent pauses for approval
Headless allowlist	STRANDS_ROS2_PUBLISH_ALLOW=/cmd_vel,/follow_path	Per-topic pre-approval, everything else still gated
Fully trusted	BYPASS_TOOL_CONSENT=true	All blocked topics pass with WARNING log

When no tool_context is available (outside an agent loop), the gate fails closed with an actionable error suggesting the env vars.

Design rationale

docs/security.md §1 documents tool scoping as the primary mitigation — agents that don't need use_ros never receive it. But agents that legitimately need use_ros for subscribe/observe can be prompt-injected into publishing velocity commands. This adds defense-in-depth: even with use_ros loaded, safety-critical topics require explicit operator consent.

Test plan

• All 12 default-blocklist topics blocked (exact + namespaced)
• Substring false positives don't match
• STRANDS_ROS2_PUBLISH_ALLOW per-topic allowlist skips the gate
• BYPASS_TOOL_CONSENT=true allows with WARNING
• tool_context.interrupt() called with correct reason dict
• Declined approval returns error
• RuntimeError (no agent loop) fails closed
• Non-blocked topics pass without gate

---

🤖 Generated with Claude Code

[cagataycali]

Reviewer note (contributor, non-gating). Reasonable defense-in-depth: the use_ros tool's publish path only had a character-level _NAME_RE while the DDS bridge got _require_secure_command_surface, so a topic-semantic blocklist for safety-critical command topics closes a real asymmetry. The namespace-stripping match (/my_robot/cmd_vel -> /cmd_vel) is the right instinct.

Before merge-ready, per AGENTS.md ("Pin every reviewed fix with a regression test"): the 44-line addition has no test file. Please commit the plan as tests: publish to /cmd_vel blocked; /my_robot/cmd_vel blocked (namespace stripping); /my_custom_topic passes; subscribe/read to /cmd_vel still works (the read path must be unaffected — this is the most important regression to pin, since over-blocking would break legitimate telemetry); and STRANDS_ROS2_PUBLISH_BLOCKLIST="" disables. One edge worth a test: namespace stripping should not let /cmd_vel_evil or /foo/notcmd_vel slip through as a substring — assert exact final-segment match, not a contains-check. CI still pending; early, non-blocking feedback.

[CurrentlyAWey]

Thanks for the review — tests added in the latest push.

tests/test_use_ros_publish_blocklist.py pins:

• All default blocked topics: /cmd_vel, /joint_command, /joint_trajectory, /emergency_stop, /navigate_to_pose, etc.
• Namespace stripping: /my_robot/cmd_vel, /ns1/ns2/cmd_vel, /fleet/robot1/emergency_stop all correctly caught
• Substring false positives: /cmd_vel_evil, /my_robot/cmd_vel_evil, /not_cmd_vel, /foo/notcmd_vel, /joint_trajectory_status, /emergency_stop_status all correctly PASS (exact final-segment match via rsplit('/', 1), not substring)
• Non-blocked topics pass: /my_custom_topic, /robot/status, /diagnostics, /tf, /rosout
• Env override: empty STRANDS_ROS2_PUBLISH_BLOCKLIST="" disables entirely; custom list replaces default
• Multi-segment entry: /joint_trajectory_controller/joint_trajectory matched both directly and as exact match in the default list

The read/subscribe path is unaffected — the blocklist check is only inside if action == "publish":. A regression test for subscribe-to-blocked-topic passing would need rclpy mocking (the subscribe path calls rclpy.create_subscription); flagging that as a follow-up if you'd like it.

[cagataycali]

Hey @CurrentlyAWey — first, genuinely thank you for this. Both PRs surface real risks (Hydra "last value wins" + agent-driven publish to /cmd_vel) and the CWE mapping + PoC quality is excellent. This is the kind of security work the project needs. 🙏

That said, we'd like to shift the enforcement model before merging, and we'd love your help doing it. Below is the reasoning — please push back if you disagree.

The concern with hard-block

Both PRs currently raise ValueError on blocklisted flags/topics. That's the correct behavior for a library used by humans, but strands-labs/robots is agent-first — the primary consumer of these tools is an LLM under strands.Agent(tools=[...]). In that setting a hard-raise has two failure modes:

1. Kills legitimate use cases entirely. A user who says "train my policy and push it to my own wandb project" or "drive the SO-100 to the pickup pose via /cmd_vel" has a completely legitimate need. With this PR they can never use the tool from an agent at all — the only escape hatch is setting an env var before the process starts, which the agent itself can't do. So the tool effectively becomes unusable for its main use case.
2. The env-override is coarse. STRANDS_TRAIN_EXTRA_FLAGS_BLOCKLIST="" / STRANDS_ROS2_PUBLISH_BLOCKLIST="" is all-or-nothing. Either you accept every risk or you accept none. There's no per-call approval.

What we'd prefer: Human-in-the-loop gate

strands-agents has first-class support for this — either via hooks (BeforeToolInvocationEvent) or via the built-in BYPASS_TOOL_CONSENT pattern. Concretely, when the agent tries to publish to /cmd_vel or pass --wandb.enable=true, we want:

• ✅ In interactive / dev mode → prompt the user: "Agent wants to publish Twist(linear.x=5.0) to /cmd_vel. Approve? [y/N]"
• ✅ In headless / production mode → still blockable via env, but on a per-flag / per-topic allowlist the operator sets once, not a global kill-switch
• ✅ In fully-trusted mode (BYPASS_TOOL_CONSENT=true) → pass through with a WARNING log

The change is small: instead of raise ValueError, call into a HIL gate helper (something like require_human_approval(reason=..., action=..., default_deny=True)) that returns a bool. On deny we still raise; on approve we proceed and audit-log.

Concrete asks

If you're up for it, could you refactor along these lines?

1. Keep the blocklist data (_BLOCKED_FLAGS, _BLOCKED_TOPICS) exactly as you have it — that catalogue is the valuable part.
2. Replace raise ValueError(...) with a call to a shared HIL helper. Happy to sketch one in a follow-up commit if useful — the strands BeforeToolInvocationEvent hook is probably the cleanest anchor point.
3. Env vars become allowlists on top of the default-deny gate rather than a global bypass. e.g. STRANDS_ROS2_PUBLISH_ALLOW="/cmd_vel" means /cmd_vel skips the prompt; everything else in the blocklist still asks.
4. Tests: assert the gate is called with the right args, rather than asserting a raise.

If reshaping the PR isn't practical, we can also land your current version behind a feature flag (default off) and iterate — happy either way.

Again, thank you for the thorough writeup and the PoCs. The security surface you identified is real; we just want the mitigation to keep the tools usable for the 99% legitimate case while still catching the injection path.

cc @cagataycali

[CurrentlyAWey]

Refactored in 6f17383: replaced the hard error return with a human-in-the-loop gate using tool_context.interrupt(), following the pattern from robot_mesh.py.

What changed:

• @tool → @tool(context=True) so use_ros receives tool_context
• _is_publish_blocked() stays as the pure blocklist checker (data unchanged)
• New _gate_publish(topic, tool_context) orchestrates the full gate:
  • Per-topic allowlist (STRANDS_ROS2_PUBLISH_ALLOW=/cmd_vel): pre-approve individual topics without a prompt
  • BYPASS_TOOL_CONSENT=true: pass all blocked topics with a WARNING log (fully-trusted mode)
  • Otherwise: tool_context.interrupt("use_ros-publish-approval", reason={...}) pauses the agent loop for operator approval
  • No tool_context: fail-closed with an actionable error (set the allowlist env or BYPASS_TOOL_CONSENT)
• Old env var STRANDS_ROS2_PUBLISH_BLOCKLIST (global override) → STRANDS_ROS2_PUBLISH_ALLOW (per-topic allowlist on top of default-deny)
• Tests now mock tool_context.interrupt() and assert the gate is called with the right args (approval, decline, RuntimeError, allowlist, bypass)

[cagataycali]

Verified and taking this over the line. The publish HIL gate mirrors the merged extra_flags gate (#1085): safety-critical command topics (cmd_vel and its namespaced forms, joint_command/joint_trajectory, e-stop, motor enable/disable, navigate_to_pose/follow_path) are gated behind tool_context.interrupt(), with a per-topic STRANDS_ROS2_PUBLISH_ALLOW allowlist, BYPASS_TOOL_CONSENT warn-and-proceed, and fail-closed when no tool_context is available. The read/echo/subscribe path is untouched. Namespace stripping via exact final-segment match (rsplit) correctly catches /my_robot/cmd_vel while /cmd_vel_evil, /not_cmd_vel and /joint_trajectory_status pass.

Brought the tests to green: (1) made TestGatePublish hermetic to the ambient BYPASS_TOOL_CONSENT/STRANDS_ROS2_PUBLISH_ALLOW env via a class-scoped autouse fixture, matching the fix applied to the extra_flags gate tests (#1246) so the suite is deterministic regardless of the shell; (2) merged main to pick up the pyarrow py.typed mypy override (#1240); (3) moved two incidental publish tests in tests/tools/test_use_ros.py off the now-gated cmd_vel topics so they exercise the marshaling / missing-arg paths they are meant to cover rather than the gate. Full lint + test suite green. Thanks for the thorough writeup and PoCs.

[cagataycali]

All checks are green and this is approved. Summary of what was needed to bring it over the line:

1. Test hermeticity (matches test(lerobot_train): make extra_flags gate tests hermetic to ambient consent env #1246): TestGatePublish cases that assert the no-context / non-covered-allowlist / interrupt paths were short-circuited by an ambient BYPASS_TOOL_CONSENT=true (and STRANDS_ROS2_PUBLISH_ALLOW), so the gate silently allowed and the tests failed with assert None is not None. Added a class-scoped autouse fixture that clears both per test; cases exercising those paths still opt in via monkeypatch.setenv.
2. pyarrow mypy drift (fix(mypy): silence pyarrow import-untyped after it dropped its py.typed marker #1240): the branch predated the pyarrow ignore_missing_imports override, so mypy reported 8 import-untyped errors. Merged main.
3. Pre-existing test interaction: the new gate correctly blocks /turtle1/cmd_vel, which two incidental publish tests in tests/tools/test_use_ros.py used as round-trip / missing-arg examples. Moved them to non-command topics so they exercise the marshaling / missing-arg paths they cover; the gate's behavior is covered by tests/test_use_ros_publish_blocklist.py.

Full lint + suite green. The repository ruleset requires an approval from someone other than the last pusher, so this needs a merge from a second maintainer (or a re-push from @CurrentlyAWey). @yinsong1986 could you do the honors?

[cagataycali]

All checks green (call-test-lint, CodeQL, breaking-change, LLM-input-safety, dependency-review). The HIL publish gate is sound: blocklist of safety-critical command surfaces (cmd_vel/joint/e-stop/nav), namespace-stripped matching, STRANDS_ROS2_PUBLISH_ALLOW + BYPASS_TOOL_CONSENT escape hatches, fail-closed with no tool_context, and a flat approve/deny interrupt sentinel that never echoes the operator reply. Mirrors the merged lerobot_train HIL gate. Approving post-rebase.

[cagataycali]

All 8 checks are green on the latest head (call-test-lint, CodeQL, Detect Breaking Changes, LLM Input Safety, dependency-review, Analyze python/actions). The publish gate has been reviewed and approved: a blocklist of safety-critical command surfaces (/cmd_vel, joint command/trajectory, e-stop, motor enable/disable, nav goals), namespace-stripped matching so /robot1/cmd_vel is also gated, STRANDS_ROS2_PUBLISH_ALLOW per-topic pre-approve + BYPASS_TOOL_CONSENT escape hatches, fail-closed when no tool_context, and a flat approve/deny interrupt sentinel that never echoes the operator reply. Mirrors the merged lerobot_train HIL gate.

This is merge-ready but the repository ruleset requires the approving review to come from someone other than the last pusher, and I am recorded as the last pusher (from the earlier test-fix commits). Routing to @yinsong1986 for the final approve + merge.