feat(sandbox): add explicit Docker and Railway user sandbox profiles - #7214
Merged
Merged
Conversation
|
🚅 Deployed to the ironclaw-pr-7214 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7214
August 5, 2026 07:56
Destroyed
github-actions
Bot
added
scope: sandbox
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds user-scoped Docker and Railway sandbox execution, two hosted sandbox profiles, hardened worker configuration, runtime policy wiring, integration tests, and conditional CI execution. ChangesUser sandbox execution
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7214
August 5, 2026 07:56
Destroyed
Contributor
There was a problem hiding this comment.
Pull request overview
Adds explicit user-sandbox execution profiles (local Docker and Railway preview) and wires builtin.shell through an enforced sandbox process binding, with new real-Docker/full-turn integration coverage, CI lane selection, and updated operator/docs contracts.
Changes:
- Introduce explicit
hosted-single-tenant-volume-sandboxed(local Docker) and...-sandboxed-railway(Railway Sandboxes) profiles, plusUserSandboxFactoryto centralize transport construction. - Shift sandbox persistence identity to
{tenant_id, user_id}and tighten worker posture (non-root, read-only rootfs,--network none, bounded logs/tmpfs/pids/cpu). - Add real Docker contract + full-turn integration test, Railway transport tests/runbook, and a dedicated CI lane triggered by sandbox-surface changes.
Reviewed changes
Copilot reviewed 87 out of 88 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/support/harness/profiles/sandbox_shell.rs | Adds a harness profile that builds a real sandboxed shell capability harness. |
| tests/integration/support/harness/profiles/mod.rs | Exposes the new sandbox_shell harness profile module. |
| tests/integration/support/harness/options.rs | Adds a sandboxed_shell option flag to opt into sandbox profile wiring in the harness. |
| tests/integration/support/harness/mod.rs | Builds sandbox-profile services input and binds a Docker-backed UserSandboxFactory when opted in. |
| tests/integration/support/docker_gate.rs | Adds a Docker/worker-image availability gate for local runs and CI fail-closed mode. |
| tests/integration/support/capability_backend.rs | Adds a SandboxShellTools backend selecting the sandboxed shell harness. |
| tests/integration/support/builder.rs | Adds with_sandbox_shell_tools() to route builtin.shell through the real sandbox profile. |
| tests/integration/reborn_sandbox_shell_turn.rs | Adds a full-turn integration test that asserts shell runs in a real Docker worker and persists workspace. |
| tests/integration/CLAUDE.md | Documents the one explicit Docker-required integration test and harness opt-in. |
| tests/fixtures/llm_traces/runtime_policy/hosted_dev_no_shell.json | Updates recorded trace copy to reflect “user sandbox” wording. |
| tests/e2e_trace_runtime_policy_serde.rs | Updates serde round-trip coverage to use ProcessBackendKind::UserSandbox. |
| tests/CLAUDE.md | Updates test-tier docs and counts to include the dedicated sandbox integration test. |
| scripts/ci/test_reborn_pr_test_plan.py | Adds a contract test asserting sandbox-surface diffs select the Docker CI lane. |
| scripts/ci/reborn_pr_test_plan.py | Adds sandbox-surface path mapping and emits run_sandbox_docker in the plan. |
| docs/reborn/target-architecture/CHECKLIST.md | Updates target-architecture checklist wording for UserSandboxProcessPort. |
| docs/reborn/railway-sandbox-operator.md | Adds a Railway sandbox operator runbook (preview-only). |
| docs/reborn/deploy-reborn-cli-docker.md | Updates Docker/Railway deployment guidance and distinguishes sandboxed profiles. |
| docs/reborn/contracts/runtime-profiles.md | Updates hosted-yolo wording to refer to user sandbox boundary. |
| docs/reborn/contracts/host-runtime.md | Updates host-runtime contract text for ProcessBackendKind::UserSandbox and {tenant,user} identity. |
| docs/reborn-binary.md | Updates doc wording to “user-sandbox process binding”. |
| docs/plans/composition-pubuse.snapshot | Updates composition re-exports snapshot for new sandbox factory/config and build-input helpers. |
| Dockerfile.sandbox-worker | Adds a pinned minimal Python worker image for local Docker user-sandbox tests. |
| Dockerfile | Adds a pinned Railway CLI download+checksum step and copies railway into the final image. |
| docker/reborn/entrypoint.sh | Recognizes sandboxed hosted-volume profiles for default config/volume fail-closed checks. |
| crates/ironclaw_sandbox/tests/user_sandbox_docker_live.rs | Adds real-Docker persistence/isolation test for per-user workspace. |
| crates/ironclaw_sandbox/tests/railway_sandbox_live.rs | Adds an ignored, manual Railway canary test for checkpoint persistence/isolation and token scrubbing. |
| crates/ironclaw_sandbox/src/sandbox_process/worker_spec.rs | Centralizes Docker worker security posture + CLI arg renderer and tests. |
| crates/ironclaw_sandbox/src/sandbox_process/user_key.rs | Updates user-key semantics/docs to production per-user identity. |
| crates/ironclaw_sandbox/src/sandbox_process/railway/tests.rs | Adds hermetic Railway CLI transport tests via a fake CLI implementation. |
| crates/ironclaw_sandbox/src/sandbox_process/network_allowlist.rs | Updates docs to refer to UserSandbox. |
| crates/ironclaw_sandbox/src/sandbox_process/container_identity.rs | Enforces non-root container user selection (workspace-owner by default) and adds tests. |
| crates/ironclaw_sandbox/src/sandbox_process/broker.rs | Updates docs to refer to user sandbox commands. |
| crates/ironclaw_sandbox/src/sandbox_process.rs | Switches workspace identity to {tenant,user}, tightens worker host config, blocks caller env injection, and exports Railway transport/config. |
| crates/ironclaw_sandbox/src/lib.rs | Re-exports new Railway preview types and updated sandbox surfaces. |
| crates/ironclaw_sandbox/CLAUDE.md | Updates crate wiring status doc for production builtin.shell execution through sandbox profiles. |
| crates/ironclaw_sandbox/Cargo.toml | Adds tokio sync feature needed by Railway transport/test code. |
| crates/ironclaw_runtime_policy/src/resolver.rs | Resolves hosted profiles to ProcessBackendKind::UserSandbox instead of TenantSandbox. |
| crates/ironclaw_runtime_policy/src/planner.rs | Updates planner docs/tests for UserSandbox backend selection. |
| crates/ironclaw_reborn_config/tests/profile_contract.rs | Adds stable-string and predicate contract coverage for the new profiles. |
| crates/ironclaw_reborn_config/src/profile.rs | Adds profile enum variants, parsing, display order, and storage-subdir mapping for sandboxed profiles. |
| crates/ironclaw_reborn_composition/tests/service_factory.rs | Updates tests to expect UserSandbox process backend and binding errors. |
| crates/ironclaw_reborn_composition/tests/production_runtime_trigger_poller.rs | Updates production harness wiring to use UserSandboxProcessPort. |
| crates/ironclaw_reborn_composition/tests/production_runtime_project_service.rs | Updates production harness wiring to use UserSandboxProcessPort. |
| crates/ironclaw_reborn_composition/tests/production_runtime_identity.rs | Updates production harness wiring to use UserSandboxProcessPort. |
| crates/ironclaw_reborn_composition/tests/production_runtime_automations.rs | Updates production harness wiring to use UserSandboxProcessPort. |
| crates/ironclaw_reborn_composition/tests/postgres_substrate.rs | Updates production policy helper naming and expected UserSandbox backend. |
| crates/ironclaw_reborn_composition/tests/libsql_substrate.rs | Updates production policy helper naming/errors for UserSandbox process binding. |
| crates/ironclaw_reborn_composition/tests/admin_api_e2e.rs | Updates admin harness production policy/backend to UserSandbox. |
| crates/ironclaw_reborn_composition/src/sandbox.rs | Introduces UserSandboxFactory to build local Docker or Railway preview process bindings. |
| crates/ironclaw_reborn_composition/src/runtime/tests/core.rs | Updates production-shaped runtime tests to use UserSandboxProcessPort. |
| crates/ironclaw_reborn_composition/src/runtime.rs | Reuses already-built capability policy from services instead of rebuilding in runtime. |
| crates/ironclaw_reborn_composition/src/root/profile.rs | Adds composition profile variants for the sandboxed hosted-volume profiles. |
| crates/ironclaw_reborn_composition/src/readiness.rs | Adds readiness state/diagnostic reason for sandboxed hosted-volume preview. |
| crates/ironclaw_reborn_composition/src/production_runtime_policy.rs | Renames production policy helper to with_user_sandbox_process_port and updates error mapping. |
| crates/ironclaw_reborn_composition/src/product_surface.rs | Adds operator-status mapping for sandboxed hosted-volume readiness. |
| crates/ironclaw_reborn_composition/src/memory_binding.rs | Treats sandboxed hosted-volume profiles as hosted-single-tenant for memory deployment classification. |
| crates/ironclaw_reborn_composition/src/lib.rs | Adds sandbox module, re-exports UserSandboxFactory and Railway config, updates error variants. |
| crates/ironclaw_reborn_composition/src/input.rs | Renames runtime process binding to UserSandbox and updates validation errors/messages. |
| crates/ironclaw_reborn_composition/src/factory/runtime_lane_assembly.rs | Wires user-sandbox process port into production services when present. |
| crates/ironclaw_reborn_composition/src/factory/production_backend_assembly.rs | Specializes builtin capability policy for the selected process backend (user-sandbox restrictions). |
| crates/ironclaw_reborn_composition/src/factory/capability_host_tests/approval_gates.rs | Removes a now-obsolete standalone tenant-sandbox port test and related test transport. |
| crates/ironclaw_reborn_composition/src/factory.rs | Makes capability_policy available outside test-only builds (used by runtime build). |
| crates/ironclaw_reborn_composition/src/error.rs | Updates RebornBuildError mapping for new user-sandbox binding errors. |
| crates/ironclaw_reborn_composition/src/deployment.rs | Adds sandboxed hosted-volume deployment config and resolved runtime policy helper. |
| crates/ironclaw_reborn_composition/src/builtin_capability_policy.rs | Adds for_process_backend(UserSandbox) projection to strip host fs/network effects from shell grant. |
| crates/ironclaw_reborn_composition/Cargo.toml | Adds ironclaw_sandbox dependency for sandbox process construction. |
| crates/ironclaw_reborn_cli/src/runtime/mod.rs | Wires the new CLI profiles, builds sandbox process bindings, and adds profile-selection contract tests. |
| crates/ironclaw_host_runtime/tests/support/host_runtime_harness.rs | Updates harness runtime policy expected backend to UserSandbox. |
| crates/ironclaw_host_runtime/tests/runtime_policy_planner_contract.rs | Updates planner contract assertions to UserSandbox. |
| crates/ironclaw_host_runtime/tests/host_runtime_services_contract.rs | Updates production wiring validation tests to require UserSandboxProcessPort. |
| crates/ironclaw_host_runtime/tests/first_party_coding_tools.rs | Updates post-edit-check sandbox routing tests to use the user-sandbox port/policy. |
| crates/ironclaw_host_runtime/tests/first_party_builtin_tools.rs | Updates first-party package/handler tests and shell routing tests to UserSandbox. |
| crates/ironclaw_host_runtime/src/services/production_wiring.rs | Renames production wiring component tracking field to user_sandbox_process_port. |
| crates/ironclaw_host_runtime/src/services/production_services.rs | Requires a sandbox process port when runtime policy selects UserSandbox. |
| crates/ironclaw_host_runtime/src/services/builder.rs | Renames builder setters to with_user_sandbox_process_port (+ production variant). |
| crates/ironclaw_host_runtime/src/services.rs | Plumbs user-sandbox process port through services and invocation resolver setup. |
| crates/ironclaw_host_runtime/src/process_port.rs | Renames TenantSandboxProcessPort to UserSandboxProcessPort and updates tests. |
| crates/ironclaw_host_runtime/src/post_edit_check.rs | Updates docs to refer to user sandbox backend. |
| crates/ironclaw_host_runtime/src/lib.rs | Re-exports UserSandboxProcessPort instead of tenant sandbox port. |
| crates/ironclaw_host_runtime/src/invocation_services/tests.rs | Updates resolver tests and error expectations for UserSandbox. |
| crates/ironclaw_host_runtime/src/invocation_services.rs | Renames configured sandbox port slot to user-sandbox and updates selection logic/comments. |
| crates/ironclaw_host_runtime/src/first_party_tools/mod.rs | Restricts builtin.shell effects under UserSandbox and adds helper to remove specific effects. |
| crates/ironclaw_host_api/src/runtime_policy.rs | Renames backend kind to UserSandbox with serde alias and updates docs/tests. |
| crates/ironclaw_host_api/src/process.rs | Updates kernel/runtimes layering docs for UserSandboxProcessPort and user-isolated transport semantics. |
| Cargo.toml | Registers the new integration test binary reborn_integration_sandbox_shell_turn. |
| Cargo.lock | Adds ironclaw_sandbox as a dependency where newly referenced. |
| .github/workflows/reborn-tests.yml | Adds a dedicated “sandbox-docker-tests” job and propagates run_sandbox_docker from the plan. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7214
August 5, 2026 08:04
Destroyed
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 87 out of 88 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Dockerfile.sandbox-worker:8
- The worker image sets
HOME=/home/sandbox, but the local-Docker transport now defaults to running the container as the numeric owner of the host workspace (often not uid 1000). In that case/home/sandboxis typically owned by uid 1000 and may be non-writable, causing Python tooling that writes to$HOME(pip caches, site config, etc.) to fail unexpectedly. SetHOMEto a path that remains writable for the chosen runtime uid (e.g./workspaceor/tmp).
ENV HOME=/home/sandbox
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48b3b1b25a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7214
August 5, 2026 08:14
Destroyed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/reborn/target-architecture/CHECKLIST.md (2)
418-418: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReconcile the
#7084status.The entry says the follow-up is owed after
#7084lands, but it also says#7084already fixes the issue. Use one status. If#7084has landed, mark this item complete. Otherwise, change “already fixes” to “will fix”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reborn/target-architecture/CHECKLIST.md` at line 418, Reconcile the `#7084` status in the checklist entry: if `#7084` has landed, mark the follow-up complete; otherwise change “already fixes” to “will fix.” Keep the surrounding guidance and references unchanged.
428-428: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the correct nested CLI path.
The sentence names
app/ironclaw_cli, then says the nested path iscrates/ironclaw_cli. The correct path iscrates/app/ironclaw_cli.Proposed correction
- makes `crates/ironclaw_cli` the first nested family path + makes `crates/app/ironclaw_cli` the first nested family pathAs per coding guidelines, the binary entry point is
crates/app/ironclaw_cli.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reborn/target-architecture/CHECKLIST.md` at line 428, Correct the CLI path references in the checklist entry describing the directory move: use crates/app/ironclaw_cli consistently, including the statement identifying the first nested family path, while preserving the package name and other rename details.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/internal/reborn-binary.md`:
- Around line 510-511: Update the `skills list` supported-profile documentation
and validation to include `hosted-single-tenant-volume-sandboxed` and
`hosted-single-tenant-volume-sandboxed-railway`, matching the profiles listed in
the Reborn profiles section; alternatively, remove those profiles from that
section if they are not intended to be accepted.
---
Outside diff comments:
In `@docs/reborn/target-architecture/CHECKLIST.md`:
- Line 418: Reconcile the `#7084` status in the checklist entry: if `#7084` has
landed, mark the follow-up complete; otherwise change “already fixes” to “will
fix.” Keep the surrounding guidance and references unchanged.
- Line 428: Correct the CLI path references in the checklist entry describing
the directory move: use crates/app/ironclaw_cli consistently, including the
statement identifying the first nested family path, while preserving the package
name and other rename details.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 43a27022-0d1d-47e5-86a4-f75b4a8e68f3
📒 Files selected for processing (12)
Cargo.tomlcrates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rscrates/app/ironclaw_composition/src/lib.rscrates/app/ironclaw_composition/src/product_surface.rscrates/app/ironclaw_composition/src/runtime.rscrates/kernel/ironclaw_host_runtime/src/services/builder.rscrates/kernel/ironclaw_host_runtime/tests/host_runtime_services_contract.rscrates/kernel/ironclaw_host_runtime/tests/support/host_runtime_harness.rsdocs/internal/plans/composition-pubuse.snapshotdocs/internal/reborn-binary.mddocs/reborn/target-architecture/CHECKLIST.mdscripts/ci/reborn_pr_test_plan.py
21 tasks
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/lanes/ironclaw_sandbox/src/sandbox_process.rs (1)
599-618: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDelete the now-unreachable per-key validation tail.
The early return rejects every non-empty
env. Everything after it runs only on a provably empty map, so theinto_iter().map(...)closure always yieldsVec::new()and its NUL and'='error strings are unreachable.Keeping a dead validation path is worse than having none: a future reader will assume caller env is sanitized rather than refused, and may relax the guard on top of validation that never ran.
♻️ Proposed simplification
fn validate_env(env: HashMap<String, String>) -> Result<Vec<String>, RuntimeProcessError> { if !env.is_empty() { return Err(RuntimeProcessError::ExecutionFailed( "user sandbox commands do not accept caller-provided environment variables".to_string(), )); } - env.into_iter() - .map(|(key, value)| { - reject_nul("environment variable name", &key)?; - reject_nul("environment variable value", &value)?; - if key.contains('=') || key.is_empty() { - return Err(RuntimeProcessError::ExecutionFailed( - "environment variable names must be non-empty and cannot contain '='" - .to_string(), - )); - } - Ok(format!("{key}={value}")) - }) - .collect() + Ok(Vec::new()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lanes/ironclaw_sandbox/src/sandbox_process.rs` around lines 599 - 618, Update validate_env to retain the early rejection for any non-empty environment and return an empty Vec directly for the empty-map case. Remove the unreachable into_iter/map closure and its per-key NUL, empty-name, and '=' validation logic.crates/kernel/ironclaw_host_runtime/src/invocation_services.rs (1)
374-395: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMerge the duplicated mount-scoped arms.
The new
TenantWorkspacearm is byte-equivalent to theScopedVirtualarm: requiremounts, wrapself.filesystemin aMountScopedRootFilesystem. Two copies of the same containment decision will drift the moment one side gains a check the other does not.Collapse them into one arm with a guard so the deployment gate stays explicit and the body stays single-sourced.
♻️ Proposed consolidation
- FilesystemBackendKind::ScopedVirtual => { - let mounts = - mounts.ok_or(InvocationServicesError::UnsupportedFilesystemBackend { - backend: plan.filesystem_backend, - })?; - Ok(Arc::new(MountScopedRootFilesystem::new( - Arc::clone(&self.filesystem), - mounts.clone(), - ))) - } - FilesystemBackendKind::TenantWorkspace - if matches!(plan.deployment, DeploymentMode::HostedMultiTenant) => - { - let mounts = - mounts.ok_or(InvocationServicesError::UnsupportedFilesystemBackend { - backend: plan.filesystem_backend, - })?; - Ok(Arc::new(MountScopedRootFilesystem::new( - Arc::clone(&self.filesystem), - mounts.clone(), - ))) - } + // Both backends resolve to the same mount-scoped view; only the + // admissible deployment differs. + FilesystemBackendKind::ScopedVirtual + | FilesystemBackendKind::TenantWorkspace + if matches!(plan.filesystem_backend, FilesystemBackendKind::ScopedVirtual) + || matches!(plan.deployment, DeploymentMode::HostedMultiTenant) => + { + let mounts = + mounts.ok_or(InvocationServicesError::UnsupportedFilesystemBackend { + backend: plan.filesystem_backend, + })?; + Ok(Arc::new(MountScopedRootFilesystem::new( + Arc::clone(&self.filesystem), + mounts.clone(), + ))) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/kernel/ironclaw_host_runtime/src/invocation_services.rs` around lines 374 - 395, Merge the `FilesystemBackendKind::ScopedVirtual` and hosted `FilesystemBackendKind::TenantWorkspace` match arms in the filesystem backend dispatch into one guarded arm. Keep the explicit `DeploymentMode::HostedMultiTenant` condition for `TenantWorkspace`, require `mounts`, and retain the single `MountScopedRootFilesystem` construction using `self.filesystem`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/app/ironclaw_composition/src/runtime.rs`:
- Around line 520-521: Update the RuntimeProcessError::UserSandboxShutdown
variant to wrap a RuntimeProcessError rather than a String, preserving the
original sandbox shutdown source error at this boundary while retaining
sanitized messaging at the product boundary.
In `@crates/app/ironclaw_composition/src/sandbox.rs`:
- Around line 22-24: Update both RebornScopedSandboxCommandTransport::connect
construction sites in crates/app/ironclaw_composition/src/sandbox.rs at lines
22-24 and 42-44 to use the runtime-resolved network policy instead of
unconditional with_network_enabled(). Preserve broker proxy configuration for
NetworkMode::Brokered, and pass the resolved allowlist into worker
environment/configuration for NetworkMode::Allowlist, including HostedDev and
HostedYoloTenantScoped, so shell commands cannot bypass egress boundaries.
In `@crates/lanes/ironclaw_sandbox/tests/user_sandbox_docker_live.rs`:
- Around line 15-24: Extend the live sandbox isolation tests around scope() to
cover two tenants: create state as a user in tenant A, then attempt to read it
with the same or another user in tenant B, and assert the state is inaccessible.
Make the tenant an input to scope() while preserving existing user, project, and
thread parameters, and add the required caller-driven two-user cross-tenant
filesystem escape assertion without changing backend or deployment
configuration.
In `@Dockerfile`:
- Around line 22-25: Update the Dockerfile package-install step for
ca-certificates and curl to use exact versions from a reproducible Debian
snapshot, or replace the base/toolchain setup with a pinned toolchain image.
Ensure the bootstrap packages used to fetch the Railway CLI cannot change across
rebuilds.
In `@scripts/ci/reborn_pr_test_plan.py`:
- Around line 159-162: Update SANDBOX_DOCKER_EXACT_PATHS in
reborn_pr_test_plan.py to derive Cargo.toml and src/lib.rs paths from
crate_directory("ironclaw_sandbox", ROOT), so relocated crate roots remain
covered. In scripts/ci/test_reborn_pr_test_plan.py lines 536-556, extend the
inventory-relocation regression with relocated Cargo.toml and src/lib.rs cases.
In `@tests/integration/CLAUDE.md`:
- Around line 80-85: Update the target name in the “Zero setup by default”
section of tests/integration/CLAUDE.md from reborn_sandbox_shell_turn to the
registered reborn_integration_sandbox_shell_turn target, preserving the existing
Docker and CI behavior description.
---
Outside diff comments:
In `@crates/kernel/ironclaw_host_runtime/src/invocation_services.rs`:
- Around line 374-395: Merge the `FilesystemBackendKind::ScopedVirtual` and
hosted `FilesystemBackendKind::TenantWorkspace` match arms in the filesystem
backend dispatch into one guarded arm. Keep the explicit
`DeploymentMode::HostedMultiTenant` condition for `TenantWorkspace`, require
`mounts`, and retain the single `MountScopedRootFilesystem` construction using
`self.filesystem`.
In `@crates/lanes/ironclaw_sandbox/src/sandbox_process.rs`:
- Around line 599-618: Update validate_env to retain the early rejection for any
non-empty environment and return an empty Vec directly for the empty-map case.
Remove the unreachable into_iter/map closure and its per-key NUL, empty-name,
and '=' validation logic.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3a154b65-bfa5-4a6b-b3a4-5a592aa606f7
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.locktests/fixtures/llm_traces/runtime_policy/hosted_dev_no_shell.jsonis excluded by!tests/fixtures/**
📒 Files selected for processing (96)
.env.example.github/workflows/reborn-tests.ymlCargo.tomlDockerfileDockerfile.sandbox-workercrates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rscrates/app/ironclaw_architecture_tests/tests/reborn_restructure_baselines.rscrates/app/ironclaw_architecture_tests/tests/reborn_struct_test_support_ratchet.rscrates/app/ironclaw_cli/src/commands/skills.rscrates/app/ironclaw_cli/src/runtime/mod.rscrates/app/ironclaw_cli/tests/smoke.rscrates/app/ironclaw_composition/Cargo.tomlcrates/app/ironclaw_composition/src/builtin_capability_policy.rscrates/app/ironclaw_composition/src/deployment.rscrates/app/ironclaw_composition/src/error.rscrates/app/ironclaw_composition/src/factory.rscrates/app/ironclaw_composition/src/factory/capability_host_tests/approval_gates.rscrates/app/ironclaw_composition/src/factory/production_backend_assembly.rscrates/app/ironclaw_composition/src/factory/runtime_lane_assembly.rscrates/app/ironclaw_composition/src/factory/tests.rscrates/app/ironclaw_composition/src/input.rscrates/app/ironclaw_composition/src/lib.rscrates/app/ironclaw_composition/src/memory_binding.rscrates/app/ironclaw_composition/src/product_surface.rscrates/app/ironclaw_composition/src/production_runtime_policy.rscrates/app/ironclaw_composition/src/readiness.rscrates/app/ironclaw_composition/src/root/profile.rscrates/app/ironclaw_composition/src/runtime.rscrates/app/ironclaw_composition/src/runtime/capability_host/tests.rscrates/app/ironclaw_composition/src/runtime/tests/core.rscrates/app/ironclaw_composition/src/sandbox.rscrates/app/ironclaw_composition/tests/admin_api_e2e.rscrates/app/ironclaw_composition/tests/libsql_substrate.rscrates/app/ironclaw_composition/tests/postgres_substrate.rscrates/app/ironclaw_composition/tests/production_runtime_automations.rscrates/app/ironclaw_composition/tests/production_runtime_identity.rscrates/app/ironclaw_composition/tests/production_runtime_project_service.rscrates/app/ironclaw_composition/tests/production_runtime_trigger_poller.rscrates/app/ironclaw_composition/tests/service_factory.rscrates/app/ironclaw_config/src/profile.rscrates/app/ironclaw_config/tests/profile_contract.rscrates/contracts/ironclaw_host_api/src/process.rscrates/contracts/ironclaw_host_api/src/runtime_policy.rscrates/kernel/ironclaw_host_runtime/src/first_party_tools/mod.rscrates/kernel/ironclaw_host_runtime/src/invocation_services.rscrates/kernel/ironclaw_host_runtime/src/invocation_services/tests.rscrates/kernel/ironclaw_host_runtime/src/lib.rscrates/kernel/ironclaw_host_runtime/src/post_edit_check.rscrates/kernel/ironclaw_host_runtime/src/process_port.rscrates/kernel/ironclaw_host_runtime/src/services.rscrates/kernel/ironclaw_host_runtime/src/services/builder.rscrates/kernel/ironclaw_host_runtime/src/services/production_services.rscrates/kernel/ironclaw_host_runtime/src/services/production_wiring.rscrates/kernel/ironclaw_host_runtime/tests/first_party_builtin_tools.rscrates/kernel/ironclaw_host_runtime/tests/first_party_coding_tools.rscrates/kernel/ironclaw_host_runtime/tests/host_runtime_services_contract.rscrates/kernel/ironclaw_host_runtime/tests/runtime_policy_planner_contract.rscrates/kernel/ironclaw_host_runtime/tests/support/host_runtime_harness.rscrates/kernel/ironclaw_runtime_policy/src/planner.rscrates/kernel/ironclaw_runtime_policy/src/resolver.rscrates/lanes/ironclaw_sandbox/AGENTS.mdcrates/lanes/ironclaw_sandbox/Cargo.tomlcrates/lanes/ironclaw_sandbox/src/lib.rscrates/lanes/ironclaw_sandbox/src/sandbox_process.rscrates/lanes/ironclaw_sandbox/src/sandbox_process/broker.rscrates/lanes/ironclaw_sandbox/src/sandbox_process/connect.rscrates/lanes/ironclaw_sandbox/src/sandbox_process/container_identity.rscrates/lanes/ironclaw_sandbox/src/sandbox_process/network_allowlist.rscrates/lanes/ironclaw_sandbox/src/sandbox_process/railway.rscrates/lanes/ironclaw_sandbox/src/sandbox_process/railway/tests.rscrates/lanes/ironclaw_sandbox/src/sandbox_process/user_key.rscrates/lanes/ironclaw_sandbox/src/sandbox_process/worker_spec.rscrates/lanes/ironclaw_sandbox/tests/railway_sandbox_live.rscrates/lanes/ironclaw_sandbox/tests/user_sandbox_docker_live.rsdocker/reborn/entrypoint.shdocs/internal/plans/composition-pubuse.snapshotdocs/internal/reborn-binary.mddocs/reborn/contracts/host-runtime.mddocs/reborn/contracts/runtime-profiles.mddocs/reborn/deploy-reborn-cli-docker.mddocs/reborn/railway-sandbox-operator.mddocs/reborn/target-architecture/CHECKLIST.mdscripts/ci/composition-budget.tomlscripts/ci/reborn_pr_test_plan.pyscripts/ci/test_reborn_pr_test_plan.pytests/CLAUDE.mdtests/e2e_trace_runtime_policy_serde.rstests/integration/CLAUDE.mdtests/integration/reborn_sandbox_shell_turn.rstests/integration/support/builder.rstests/integration/support/capability_backend.rstests/integration/support/docker_gate.rstests/integration/support/harness/mod.rstests/integration/support/harness/options.rstests/integration/support/harness/profiles/mod.rstests/integration/support/harness/profiles/sandbox_shell.rs
💤 Files with no reviewable changes (1)
- crates/app/ironclaw_composition/src/factory/capability_host_tests/approval_gates.rs
Comment on lines
+22
to
+24
| let transport = RebornScopedSandboxCommandTransport::connect( | ||
| RebornSandboxConfig::new(workspace_root).with_network_enabled(), | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline crates/lanes/ironclaw_sandbox/src/sandbox_process.rs --items all
ast-grep outline crates/lanes/ironclaw_sandbox/src/sandbox_process/railway.rs --items all
rg -n -C 6 \
'with_network_enabled|network_enabled|NetworkMode|network_policy|allowlist|allowed_domains' \
crates/lanes/ironclaw_sandbox \
crates/app/ironclaw_composition \
crates/kernel/ironclaw_runtime_policy \
crates/kernel/ironclaw_host_runtime
rg -n -C 5 \
'build_(local_docker|railway)_user_sandbox_binding|with_(production_)?user_sandbox_process_port' \
crates \
testsRepository: nearai/ironclaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- sandbox.rs ---\n'
cat -n crates/app/ironclaw_composition/src/sandbox.rs
printf '\n--- sandbox_process relevant symbols ---\n'
rg -n -C 8 'enum .*Network|struct .*Config|with_network_enabled|network_enabled|NetworkMode|execute|shell|command|docker|railway' crates/lanes/ironclaw_sandbox/src/sandbox_process.rs crates/lanes/ironclaw_sandbox/src/sandbox_process/railway.rs
printf '\n--- sandbox_process.rs slice ---\n'
sed -n '1,260p' crates/lanes/ironclaw_sandbox/src/sandbox_process.rs
printf '\n--- railway.rs slice ---\n'
sed -n '1,260p' crates/lanes/ironclaw_sandbox/src/sandbox_process/railway.rsRepository: nearai/ironclaw
Length of output: 244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- sandbox.rs ---'
cat -n crates/app/ironclaw_composition/src/sandbox.rs
printf '%s\n' ''
printf '%s\n' '--- sandbox_process relevant symbols ---'
rg -n -C 8 'enum .*Network|struct .*Config|with_network_enabled|network_enabled|NetworkMode|execute|shell|command|docker|railway' crates/lanes/ironclaw_sandbox/src/sandbox_process.rs crates/lanes/ironclaw_sandbox/src/sandbox_process/railway.rs
printf '%s\n' ''
printf '%s\n' '--- sandbox_process.rs slice ---'
sed -n '1,260p' crates/lanes/ironclaw_sandbox/src/sandbox_process.rs
printf '%s\n' ''
printf '%s\n' '--- railway.rs slice ---'
sed -n '1,260p' crates/lanes/ironclaw_sandbox/src/sandbox_process/railway.rsRepository: nearai/ironclaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- allowed domains config functions ---'
rg -n -C 10 'sandbox_allowed_domains|sandbox_network_policy|with_network_enabled|with_network_allowlist|allowed_domains|NetworkPolicy' crates/lanes/ironclaw_sandbox/src/sandbox_process crates/lanes/ironclaw_sandbox/src/sandbox_process -g '*.rs' | sed -n '1,240p'
printf '%s\n' ''
printf '%s\n' '--- Railway config methods and construction ---'
rg -n -C 8 'impl RailwayPreviewSandboxConfig|struct RailwayPreviewSandboxConfig|with_network_enabled|network_enabled|NetworkBroker|allowed_domains|command_env|container_network_mode|execute' crates/lanes/ironclaw_sandbox/src/sandbox_process/railway.rs crates/lanes/ironclaw_sandbox/src/sandbox_process.rs | sed -n '1,260p'
printf '%s\n' ''
printf '%s\n' '--- tests around DirectAllowlist/networking config ---'
sed -n '1688,1738p' crates/app/ironclaw_composition/src/factory/tests.rs
rg -n -C 8 'DefaultSandboxNetworkEgress|DIRECT|allowlist|WebAccessEgress' crates/lanes/ironclaw_sandbox -g '*.rs'Repository: nearai/ironclaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Railway sandbox argv construction ---'
sed -n '1,140p' crates/lanes/ironclaw_sandbox/src/sandbox_process/railway.rs
rg -n -C 7 'fn sandbox_exec_argv|fn ephemeral_worker_argv|--exec|--network|direct|allowed_domains|network_mode_env|IRCL|IRONCLAW' crates/lanes/ironclaw_sandbox/src/sandbox_process/railway.rs
printf '%s\n' ''
printf '%s\n' '--- user-sandbox builder call sites in composition ---'
rg -n -C 12 'with_user_sandbox|user_sandbox_network_policy|sandbox_network_policy|user_sandbox_process_backend|ProcessBackendKind::UserSandbox' crates/app/ironclaw_composition crates/kernel/ironclaw_host_runtime crates/app/ironclaw_composition/src -g '*.rs'
printf '%s\n' ''
printf '%s\n' '--- docker worker launch test around network disabled/allowed env ---'
sed -n '880,960p' crates/lanes/ironclaw_sandbox/src/sandbox_process.rsRepository: nearai/ironclaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- local production policy construction ---'
rg -n -C 12 'RebornProductionRuntimePolicy|with_user_sandbox_process_port|LocalSingleUser|Allowlist|DirectAllowlist|HostedDev|HostedYoloTenantScoped|HostedSafe|network_mode' crates/app/ironclaw_composition/src crates/kernel/ironclaw_runtime_policy/src crates/kernel/ironclaw_host_runtime/src/runtime -g '*.rs'
printf '%s\n' ''
printf '%s\n' '--- runtime policy policy construction helpers ---'
rg -n -C 10 'fn .*_user_sandbox|with_user_sandbox|NetworkMode|DefaultSandboxNetworkEgress|ALLOWED|allowed|policy|egress' crates/kernel/ironclaw_runtime_policy/src -g '*.rs'Repository: nearai/ironclaw
Length of output: 50374
Wire the resolved user-sandbox network policy into with_network_enabled() paths.
crates/app/ironclaw_composition/src/sandbox.rs#L22-L24 and crates/app/ironclaw_composition/src/sandbox.rs#L42-L44 build the Docker/Railway user-sandbox transport with direct egress (with_network_enabled()) but do not pass the policy selected by the runtime profile. For NetworkMode::Brokered, keep the container behind the broker proxy configuration instead of enabling direct networking. For NetworkMode::Allowlist (HostedDev, HostedYoloTenantScoped), pass the resolved allowlist into the worker env/config so shell commands cannot bypass the declared egress boundaries.
📍 Affects 1 file
crates/app/ironclaw_composition/src/sandbox.rs#L22-L24(this comment)crates/app/ironclaw_composition/src/sandbox.rs#L42-L44
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/app/ironclaw_composition/src/sandbox.rs` around lines 22 - 24, Update
both RebornScopedSandboxCommandTransport::connect construction sites in
crates/app/ironclaw_composition/src/sandbox.rs at lines 22-24 and 42-44 to use
the runtime-resolved network policy instead of unconditional
with_network_enabled(). Preserve broker proxy configuration for
NetworkMode::Brokered, and pass the resolved allowlist into worker
environment/configuration for NetworkMode::Allowlist, including HostedDev and
HostedYoloTenantScoped, so shell commands cannot bypass egress boundaries.
Sources: Coding guidelines, Path instructions
Collaborator
Author
There was a problem hiding this comment.
Valid policy/implementation mismatch. PR1 intentionally enables direct egress for this preview (documented and covered by live tests), while the selected policy resolves . The suggested generic factory branching is not safe because Railway/local broker and allowlist enforcement are not implemented here; it would either break the preview or imply enforcement that does not exist. Leaving this open for the explicit contract choice: introduce a direct-egress preview policy or restore deny/brokered egress.
There was a problem hiding this comment.
@henrypark133, agreed. The finding is valid, but the generic factory change is not a safe fix.
The current behavior intentionally provides direct egress. The selected runtime policy does not describe that behavior. This conflicts with the repository invariant: “Do not weaken … network … allowlists.”
Keep this finding open until the contract makes one explicit choice:
- Add a preview-only direct-egress runtime policy and select it for both sandbox profiles; or
- Implement brokered and allowlist enforcement in the Docker and Railway transports, then retain the current policy modes.
Do not add policy branching that claims enforcement before the transports enforce it.
✏️ Learnings added
Learnt from: henrypark133
Repo: nearai/ironclaw PR: 7214
File: crates/app/ironclaw_composition/src/sandbox.rs:22-24
Timestamp: 2026-08-07T21:13:16.351Z
Learning: For the Docker and Railway user-sandbox preview profiles in `crates/app/ironclaw_composition/src/sandbox.rs`, PR `#7214` intentionally enables direct outbound egress. The selected runtime policy currently resolves to a different egress mode, and Docker/Railway brokered or allowlist enforcement is not implemented. Do not add generic policy branching that implies egress enforcement until the transport implements it; resolve the mismatch with an explicit direct-egress preview policy or with real broker/allowlist enforcement.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/ci/reborn_pr_test_plan.py (1)
728-738: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExtend Docker-lane selectors to
ironclaw_composition/src/runtime.rs.CLAUDE.md/AGENTS.md require integration-first coverage for production-wired Reborn behavior.
SANDBOX_DOCKER_EXACT_PATHSdoes not includecrates/app/ironclaw_composition/src/runtime.rs, so a PR touchingRebornRuntime::UserSandboxShutdowncan skiprun_sandbox_docker. Add the exact path to the selector inventory and a regression asserting the non-noneplan withrun_sandbox_dockerenabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/reborn_pr_test_plan.py` around lines 728 - 738, Update SANDBOX_DOCKER_EXACT_PATHS in scripts/ci/reborn_pr_test_plan.py to include crates/app/ironclaw_composition/src/runtime.rs, ensuring changes to RebornRuntime::UserSandboxShutdown enable run_sandbox_docker. Add a regression test in scripts/ci/test_reborn_pr_test_plan.py covering that exact path and asserting a non-none plan with run_sandbox_docker enabled.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@scripts/ci/reborn_pr_test_plan.py`:
- Around line 728-738: Update SANDBOX_DOCKER_EXACT_PATHS in
scripts/ci/reborn_pr_test_plan.py to include
crates/app/ironclaw_composition/src/runtime.rs, ensuring changes to
RebornRuntime::UserSandboxShutdown enable run_sandbox_docker. Add a regression
test in scripts/ci/test_reborn_pr_test_plan.py covering that exact path and
asserting a non-none plan with run_sandbox_docker enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eecdb945-f12e-4a16-b018-2a265f29ad32
📒 Files selected for processing (6)
crates/app/ironclaw_composition/src/runtime.rscrates/app/ironclaw_composition/src/runtime/tests/core.rscrates/lanes/ironclaw_sandbox/tests/user_sandbox_docker_live.rsscripts/ci/reborn_pr_test_plan.pyscripts/ci/test_reborn_pr_test_plan.pytests/integration/CLAUDE.md
…box-railway # Conflicts: # crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs # crates/app/ironclaw_architecture_tests/tests/reborn_restructure_baselines.rs # crates/kernel/ironclaw_host_runtime/tests/first_party_builtin_tools.rs # scripts/ci/composition-budget.toml # tests/CLAUDE.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/app/ironclaw_composition/src/factory.rs (1)
208-208: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the crate-private re-export.
LateBoundAgentTurnRuntimeis only referenced insidecrates/app/ironclaw_composition/src/factory, andfactoryitself is a private Rust module. This path-preservation shim violates: “Usepub useonly for an architecture-mandated contract facade or when exposing a type to downstream consumers; never use it as a path-preservation shim.” Import it fromfactory::trigger_creation_assembly::LateBoundAgentTurnRuntimeinstead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/app/ironclaw_composition/src/factory.rs` at line 208, Remove the crate-private re-export of LateBoundAgentTurnRuntime from factory.rs, and update its internal references to import it directly from factory::trigger_creation_assembly::LateBoundAgentTurnRuntime.Sources: Coding guidelines, Path instructions
♻️ Duplicate comments (1)
scripts/ci/reborn_pr_test_plan.py (1)
159-187: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSchedule real-Docker validation for
factory.rssandbox wiring.
crates/app/ironclaw_composition/src/factory.rschangesRebornRuntimeStores.user_sandbox_process_portand production capability-policy availability. It is absent from the Docker selector and its regression matrix. A diff limited to that file can skip the enforced Docker lane.
scripts/ci/reborn_pr_test_plan.py#L159-L187: addcrates/app/ironclaw_composition/src/factory.rstoSANDBOX_DOCKER_EXACT_PATHS.scripts/ci/test_reborn_pr_test_plan.py#L527-L563: add the same path totest_user_sandbox_worker_change_selects_real_docker_lane.As per path instructions, “CI and dev tooling” scripts gate merges, so behavior changes need matching workflow updates.
#!/bin/bash set -euo pipefail rg -n -C2 \ 'SANDBOX_DOCKER_EXACT_PATHS|crates/app/ironclaw_composition/src/factory\.rs' \ scripts/ci/reborn_pr_test_plan.py scripts/ci/test_reborn_pr_test_plan.py🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/reborn_pr_test_plan.py` around lines 159 - 187, Add crates/app/ironclaw_composition/src/factory.rs to SANDBOX_DOCKER_EXACT_PATHS in scripts/ci/reborn_pr_test_plan.py and add the same path to test_user_sandbox_worker_change_selects_real_docker_lane in scripts/ci/test_reborn_pr_test_plan.py, ensuring factory.rs-only changes select the real-Docker validation lane.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/app/ironclaw_composition/src/factory.rs`:
- Line 208: Remove the crate-private re-export of LateBoundAgentTurnRuntime from
factory.rs, and update its internal references to import it directly from
factory::trigger_creation_assembly::LateBoundAgentTurnRuntime.
---
Duplicate comments:
In `@scripts/ci/reborn_pr_test_plan.py`:
- Around line 159-187: Add crates/app/ironclaw_composition/src/factory.rs to
SANDBOX_DOCKER_EXACT_PATHS in scripts/ci/reborn_pr_test_plan.py and add the same
path to test_user_sandbox_worker_change_selects_real_docker_lane in
scripts/ci/test_reborn_pr_test_plan.py, ensuring factory.rs-only changes select
the real-Docker validation lane.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 19206401-20a6-4625-abea-d0057f592611
📒 Files selected for processing (19)
crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rscrates/app/ironclaw_architecture_tests/tests/reborn_restructure_baselines.rscrates/app/ironclaw_composition/src/builtin_capability_policy.rscrates/app/ironclaw_composition/src/factory.rscrates/app/ironclaw_composition/src/factory/production_backend_assembly.rscrates/app/ironclaw_composition/src/factory/tests.rscrates/app/ironclaw_composition/src/product_surface.rscrates/app/ironclaw_composition/src/runtime.rscrates/app/ironclaw_composition/src/runtime/capability_host/tests.rscrates/app/ironclaw_composition/src/runtime/tests/core.rscrates/kernel/ironclaw_host_runtime/src/first_party_tools/mod.rscrates/kernel/ironclaw_host_runtime/src/lib.rscrates/kernel/ironclaw_host_runtime/tests/first_party_builtin_tools.rsscripts/ci/composition-budget.tomlscripts/ci/reborn_pr_test_plan.pyscripts/ci/test_reborn_pr_test_plan.pytests/CLAUDE.mdtests/integration/CLAUDE.mdtests/integration/support/harness/mod.rs
💤 Files with no reviewable changes (1)
- crates/app/ironclaw_composition/src/builtin_capability_policy.rs
pranavraja99
approved these changes
Aug 7, 2026
29 tasks
This was referenced Aug 8, 2026
This was referenced Aug 19, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Change Type
Linked Issue
Related #6468, #6469, #6473.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warnings(affected packages passed scoped--all-targetsclippy)cargo build(covered by clippy and test compilation)cargo test --features integrationif database-backed or integration behavior changed (Not applicable: no database behavior; the affected Reborn integration target ran directly.)Test Strategy
User behavior:
An explicit sandbox profile runs Python in its selected provider, maintains a tenant/user workspace, and leaves non-sandbox profiles unchanged. Workers receive no host credentials. Sandbox profiles explicitly grant unrestricted direct outbound network access for PR1; ad-hoc transport construction remains fail-closed with
--network none.Risk areas:
Tests added or updated:
ironclaw_sandboxunit suite (218 tests), profile/config and runtime-policy contracts, including fail-closed defaults, explicit direct-network rendering, graceful provider cleanup, and malformed-response checkpoint safety.user_sandbox.What the tests prove:
The production local profile selects the Docker-backed
user_sandboxbinding, the Railway profile selects its remote transport without connecting to Docker, and non-sandbox profiles ignore Railway configuration. Runtime-policy tests reject missing or unexpected sandbox bindings. Real-Docker tests preserve one user's workspace across turns, isolate other users, prove direct HTTPS egress without credential environment variables, return non-zero command exits as ordinary bounded sandbox results with stderr intact, and complete a real Reborn sandbox-shell turn. Hermetic Railway tests cover deterministic checkpoint restoration, credential scrubbing, timeout cleanup, exact-checkpoint provisioning, graceful shutdown cleanup, malformed-response checkpoint safety, and bounded idle-LRU user state that never evicts active entries.Commands run:
Security Impact
Yes. Shell execution moves only through a sandbox process binding. Workers have no caller environment or credentials, run non-root with a read-only root filesystem, no-new-privileges, and bounded CPU/PIDs/tmp/logs/output. Ad-hoc transports default to
--network none; only sandbox-enabled deployment profiles opt into unrestricted direct Docker/provider-NAT egress, surfaced asIRONCLAW_REBORN_NETWORK_MODE=direct. Railway CLI credentials stay host-side. The preview does not yet enforce a per-user persistent workspace disk quota. Custom CLI setup, secret-store mediation, SigV4, and restricted or credential-mediated egress are out of scope.Reborn Trust-Boundary Checklist
serde(default)fields fail closed or have migration tests: profile/config parsing rejects incomplete sandbox configuration.Database Impact
None. No schema or migration change.
Blast Radius
Runtime profile parsing, CLI boot composition, process dispatch, Docker image/CI, and deployment docs. Existing non-sandbox profiles retain their current process backend and ignore Railway settings.
Composition budget rationale: re-seed the absolute composition ratchet from 40,499 to 40,747 (+248 LOC) for provider-neutral profile/deployment mapping, fail-closed capability-policy projection, and the centralized Docker/Railway binding functions. Provider execution remains in
ironclaw_sandbox.Host API ratchet rationale: raise
ironclaw_host_apifrom 18,570 to the measured 18,799 for theUserSandboxwire rename, legacytenant_sandboxserde alias, canonical reserialization compatibility test, and provider-neutral graceful transport lifecycle hook. This is contract vocabulary and its compatibility proof; sandbox execution and cleanup remain inironclaw_sandbox.Rollback Plan
Select a non-sandbox profile or revert this PR. There is no migration; isolated workspaces/checkpoints can be retained or removed operationally.
Review Follow-Through
Railway is preview-only: use exactly one IronClaw replica and avoid deployment overlap because checkpoint coordination is process-local, not a distributed lease. Provider volume limits are the current backstop because PR1 does not enforce a per-user persistent workspace quota. PR1 is Python-only with unrestricted direct egress on sandbox-enabled profiles. Destination filtering, credential mediation, and custom CLI installation are follow-ups.
Review track: C (security/runtime/CI)