Skip to content

Commit 7dfec5e

Browse files
authored
Merge branch 'main' into fix/issue-264-pin
2 parents 4589bef + e52bd6a commit 7dfec5e

22 files changed

Lines changed: 1340 additions & 43 deletions

AGENTS.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,4 +238,17 @@ Corrections from code review that apply to all future contributions:
238238
Do not manually bump tags; merge the Dependabot PR.
239239
- **Especially `pypa/gh-action-pypi-publish`** — it uses a moving `release/v1`
240240
branch, which is exactly the supply-chain pattern that the `tj-actions/changed-files`
241-
incident exploited. This pin is non-negotiable.
241+
incident exploited. This pin is non-negotiable.
242+
243+
### Operational Runbooks for Security Pins
244+
- **A static security pin must ship with a rotation runbook, not just a recompute
245+
command.** A docstring one-liner that recomputes a pin is necessary but not
246+
sufficient; on-call at 3 AM needs a documented grace-period strategy. For the
247+
Amazon Root CA1 pin (`provision._AMAZON_ROOT_CA1_PINS`) the runbook lives in
248+
README.md > "CA Pin Rotation Runbook": dual-pin tuple during the overlap, ship
249+
the new pin first, drop the old pin in a follow-up release after fleet uptake,
250+
and use `STRANDS_MESH_CA_PINS` only as an emergency out-of-band override.
251+
- **Make the accepted-pin set a collection, never a scalar.** `_resolve_ca_pins()`
252+
returns a `frozenset` so the dual-pin grace period is expressible. Any future
253+
pinned fingerprint (other roots, signing keys) should follow the same
254+
multi-value shape so rotation never requires a flag-day deploy.

CHANGELOG.md

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,25 @@
33
All notable behavioural changes to `strands-robots` are logged here. Follows
44
[Keep a Changelog](https://keepachangelog.com/) conventions.
55

6+
## Unreleased - #320 (MuJoCo robot-scene ground-plane z-fighting)
7+
8+
### Fixed: broken floor render when a robot asset ships its own ground plane
9+
10+
Robots whose asset MJCF includes its own ground/floor plane (e.g.
11+
``franka_emika_panda/scene.xml`` ships ``<geom name="floor" type="plane"/>``)
12+
produced a **severely broken floor** - a flickering checkerboard/triangle mess
13+
- when added to a world created with ``ground_plane=True`` (the default). Two
14+
coplanar infinite ground planes at z=0 with different checker materials
15+
(``grid_mat`` vs the robot's ``groundplane``) caused depth-buffer Z-fighting.
16+
The artifact corrupted rendered videos, camera observations fed to policies,
17+
and demos, with no error raised.
18+
19+
``SpecBuilder.attach_robot`` now strips plane geoms from the robot scene MJCF
20+
before attaching it, so exactly one world-owned ``ground`` plane survives. The
21+
world ``ground`` plane (configurable via ``create_world(ground_plane=...)``)
22+
is the single source of truth; robots contribute only their own
23+
bodies/joints/actuators/sensors.
24+
625
## Unreleased - #228 (AWS IoT provisioning hardening)
726

827
### Changed: default presigned-URL TTL for camera offload
@@ -39,8 +58,7 @@ Applies to ``strands_robots.mesh.iot.provision`` and
3958
``re.fullmatch``) applied symmetrically across ``provision_robot``,
4059
``provision_operator``, and ``teardown_thing``. Rejects path
4160
separators, dots, spaces, NUL, non-ASCII, and trailing
42-
``
43-
``/````/`` ``. Pre-existing AWS IoT Things containing ``:``
61+
``\n``/``\r``/``\t``. Pre-existing AWS IoT Things containing ``:``
4462
must be renamed (we deliberately reject ``:`` due to NTFS / classic
4563
Mac filesystem semantics).
4664
- **IoT policy scope** — robot/operator policies use explicit

README.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,7 @@ agent.tool.gr00t_inference(action="stop", port=8000)
580580
| `STRANDS_MESH_AUDIT_DIR` | Directory for the safety audit log (`mesh_audit.jsonl`) | `~/.strands_robots/` |
581581
| `STRANDS_MESH_CA_PINS` | Comma-separated SHA-256 hex pins, **additive** to the bundled Amazon Root CA1 pin tuple. Operator break-glass for an AWS-side root rotation that arrives before the next `strands-robots` release ships the new pin. Must match `^[0-9a-fA-F]{64}$` per entry. | unset |
582582
| `STRANDS_MESH_DISABLE_CA_PIN` | Set to `true` (case-insensitive) to skip CA pin verification on the *download* path only. The on-disk re-use path always raw-checks the pin regardless. Last-resort break-glass; prefer `STRANDS_MESH_CA_PINS` for rotations. | `false` |
583+
| `STRANDS_MESH_CAMERA_DISABLED` | Privacy kill-switch. Set truthy (`true`/`1`/`yes`/`on`, case-insensitive) to short-circuit the mesh camera publish loop before any frame is collected, encoded, or uploaded. Invalid values raise `ValueError`. | `false` |
583584
| `STRANDS_MESH_CAMERA_PRESIGN_TTL` | Default TTL (seconds) for S3 presigned URLs emitted on the IoT camera-offload path. Capped at 3600s (1h); a `0` is clamped to 1s. Non-integer values fall back to the default with a WARNING. | `60` |
584585
| `GROOT_API_TOKEN` | API token for GR00T inference service | - |
585586
| `STRANDS_ROBOT_MODE` | Override `Robot()` factory mode detection (`sim`, `real`, `auto`) | `auto` |
@@ -592,6 +593,45 @@ agent.tool.gr00t_inference(action="stop", port=8000)
592593
| `STRANDS_GROOT_WIRE_LOG` | Path to a directory where `Gr00tPolicy` will dump pre-inference observations + post-inference action chunks as pickle files (one per `get_actions` call, named `{local,service}_call{N:04d}.pkl`). Used by the #187 bisection plan to verify whether LOCAL and SERVICE inference paths send byte-identical observations to the model. Run an eval once with each mode into the same dir, then `np.allclose` matching files. | unset |
593594
| `STRANDS_GROOT_WIRE_LOG_MAX_CALLS` | Cap on number of wire-payload dumps per process when `STRANDS_GROOT_WIRE_LOG` is set. Prevents multi-GB pickle archives on long evals. The first few calls are enough to bisect a divergence. | `10` |
594595

596+
### CA Pin Rotation Runbook
597+
598+
The IoT provisioner downloads `AmazonRootCA1.pem` over HTTPS and pins its
599+
SHA-256 fingerprint (`strands_robots/mesh/iot/provision._AMAZON_ROOT_CA1_PINS`)
600+
before trusting the bytes. Because the pin is static, an AWS-side root CA
601+
rotation needs an operational plan. On-call at 3 AM should follow this runbook,
602+
not reverse-engineer it from a docstring.
603+
604+
**Recompute the pin** (run when AWS publishes a new root, or to verify the
605+
current one):
606+
607+
```bash
608+
python -c "import hashlib, urllib.request as u; \
609+
print(hashlib.sha256(u.urlopen( \
610+
'https://www.amazontrust.com/repository/AmazonRootCA1.pem' \
611+
).read()).hexdigest())"
612+
```
613+
614+
**Grace-period strategy (dual-pin).** `_AMAZON_ROOT_CA1_PINS` is a tuple, so a
615+
rotation lands as two releases:
616+
617+
1. Ship a release whose tuple contains **both** the new pin and the old pin.
618+
`_resolve_ca_pins()` accepts any pin in the set, so fleets on the old root
619+
and fleets that have already seen the new root both verify successfully.
620+
2. Wait for fleet uptake (track your own deploy rollout; there is no flag day).
621+
3. Ship a follow-up release that drops the old pin once every fleet member has
622+
upgraded and AWS has retired the old root.
623+
624+
**Monitor for upcoming rotations.** Subscribe to AWS security bulletins
625+
(https://aws.amazon.com/security/security-bulletins/) and the Amazon Trust
626+
Services repository; AWS publishes root CA deprecation timelines ahead of time.
627+
628+
**Emergency override.** If a rotation lands faster than a code release can ship,
629+
operators stage the new pin out-of-band via `STRANDS_MESH_CA_PINS`
630+
(comma-separated 64-char lowercase hex). It is **additive** to the built-in
631+
tuple, so the old pin keeps working during the overlap. Prefer this over
632+
`STRANDS_MESH_DISABLE_CA_PIN`, which disables pinning entirely on the download
633+
path and should be a last resort.
634+
595635
### Mesh Networking
596636

597637
Every `Robot()` and `Simulation()` constructed in a process is automatically a

strands_robots/mesh/_acl_config.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,22 @@
4141

4242
logger = logging.getLogger(__name__)
4343

44+
45+
class PermissiveACLError(RuntimeError):
46+
"""Raised when an operator-supplied ACL uses the blacklist footgun
47+
(``default_permission='allow'`` with explicit rules) without opting
48+
in via ``STRANDS_MESH_ACCEPT_PERMISSIVE_ACL``. Pentest B-08 / F-14.
49+
50+
The built-in permissive default (allow + EMPTY rules) is exempt --
51+
it is gated separately by the ``Mesh.start`` refuse-to-start path
52+
(:meth:`Mesh._refuse_under_permissive_default_acl`) which fires under
53+
mTLS. This error closes the *operator-file blacklist* footgun: a
54+
hand-written ``allow + rules`` ACL is an explicit, load-bearing
55+
anti-pattern that should fail loud rather than silently expose any
56+
rule gap.
57+
"""
58+
59+
4460
#: Maximum bytes of an ACL file we will load. Anything larger is almost
4561
#: certainly an attacker probing for an OOM.
4662
ACL_FILE_MAX_BYTES: int = 256 * 1024
@@ -202,6 +218,29 @@ def _load_acl_file(path: Path) -> dict[str, Any]:
202218
path,
203219
len(data["rules"]),
204220
)
221+
# B-08 / F-14: the warning above is the first-line signal; this
222+
# is the hard gate. An operator-supplied blacklist ACL
223+
# (allow + explicit rules) is a load-bearing anti-pattern --
224+
# any gap in the rule set exposes the mesh. Refuse to load it
225+
# unless the operator has explicitly acknowledged the posture
226+
# via STRANDS_MESH_ACCEPT_PERMISSIVE_ACL. The built-in default
227+
# (allow + EMPTY rules) does not reach this branch and stays
228+
# gated by Mesh.start's refuse-to-start path.
229+
accept = os.getenv("STRANDS_MESH_ACCEPT_PERMISSIVE_ACL", "").strip().lower() in (
230+
"1",
231+
"true",
232+
"yes",
233+
)
234+
if not accept:
235+
raise PermissiveACLError(
236+
f"ACL file {path} uses default_permission='allow' with "
237+
f"{len(data['rules'])} rule(s) -- a blacklist policy where any "
238+
"rule gap exposes the mesh. Refusing to load. Remediate one of:\n"
239+
" 1. Rewrite the ACL with default_permission='deny' and "
240+
"explicit allow rules (see examples/mesh_acl_example.json5).\n"
241+
" 2. Set STRANDS_MESH_ACCEPT_PERMISSIVE_ACL=1 to acknowledge "
242+
"the dev/lab posture."
243+
)
205244
_validate_acl_shape(data, path)
206245
return data
207246

strands_robots/mesh/core.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1042,7 +1042,14 @@ def _exec_cmd(self, data: dict[str, Any]) -> None:
10421042
# {"action":"execute",...}. Outgoing send/broadcast/tell still accept
10431043
# the ergonomic dict-or-string forms because tell() wraps internally.
10441044
#
1045-
rkey = f"strands/{sender}/response/{turn}" if sender else None
1045+
# F-15 / B-09: include the responder's own peer_id as a topic
1046+
# segment so the IoT robot policy can scope publish to
1047+
# ``strands/+/response/${ThingName}/*`` -- a robot can only
1048+
# publish responses tagged with its OWN ThingName, closing the
1049+
# cross-robot response-spoof surface. The requester subscribes
1050+
# with ``response/**`` so the extra segment matches. Operator
1051+
# prefix (``{sender}``) is unchanged so routing is preserved.
1052+
rkey = f"strands/{sender}/response/{self.peer_id}/{turn}" if sender else None
10461053
if cmd is None or not isinstance(cmd, dict):
10471054
# route non-dict envelope rejection
10481055
# through the same audit + wire-response path as

strands_robots/mesh/input.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from collections.abc import Callable
2626
from typing import TYPE_CHECKING, Any
2727

28+
from strands_robots.mesh.security import ValidationError, validate_input_frame
2829
from strands_robots.mesh.session import put
2930

3031
if TYPE_CHECKING:
@@ -202,6 +203,7 @@ def __init__(
202203
self._error_count = 0
203204
self._last_seq = -1
204205
self._drops = 0
206+
self._rejected = 0
205207
self._start_time = 0.0
206208

207209
def __repr__(self) -> str:
@@ -222,6 +224,7 @@ def stats(self) -> dict[str, Any]:
222224
"frames_received": self._frame_count,
223225
"errors": self._error_count,
224226
"drops": self._drops,
227+
"rejected": self._rejected,
225228
"hz_actual": self._frame_count / elapsed if elapsed > 0 else 0,
226229
}
227230

@@ -271,7 +274,25 @@ def _on_input(self, topic: str, data: dict[str, Any]) -> None:
271274
if self._last_seq >= 0 and seq > self._last_seq + 1:
272275
self._drops += seq - self._last_seq - 1
273276
self._last_seq = seq
274-
self._apply_fn(self.robot, action)
277+
# B-04 / F-02: validate the teleop frame before it reaches
278+
# send_action(). A LAN-adjacent peer that discovers this
279+
# source peer_id could otherwise drive the follower's joints
280+
# directly with unbounded / non-finite values. validate_input_frame
281+
# bounds key count, key charset, and clamps each value to a
282+
# finite magnitude. Rejected frames are counted + logged and
283+
# dropped (never applied) rather than crashing the receiver.
284+
try:
285+
safe_action = validate_input_frame(action)
286+
except ValidationError as verr:
287+
self._rejected = getattr(self, "_rejected", 0) + 1
288+
if self._rejected <= 5:
289+
logger.warning(
290+
"[mesh] input frame rejected from %s: %s",
291+
self.source_peer_id,
292+
verr,
293+
)
294+
return
295+
self._apply_fn(self.robot, safe_action)
275296
self._frame_count += 1
276297
except Exception as exc:
277298
self._error_count += 1

0 commit comments

Comments
 (0)