fix(sandbox): close metadata permission escape paths from Seatbelt review - #14
fix(sandbox): close metadata permission escape paths from Seatbelt review#14echoVic wants to merge 8 commits into
Conversation
📝 WalkthroughWalkthroughThe PR separates protected metadata roots from ordinary working directories, adds safe metadata-write validation, commits session permission grants with interaction resolution, hardens Linux and macOS sandbox policies, and updates runtime, provider, contract, release, and UI tests. ChangesProtected metadata permission model
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/orca-tools/src/sandbox/mod.rs (1)
294-301: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMetadata grants are filtered before normalization, so non-canonical inputs are silently dropped on every platform.
is_safe_metadata_writable_rootrequirespath.canonicalize() == path, but all three filters run on the raw caller-provided path and canonicalize/normalize only afterwards — a legitimate grant expressed non-canonically (relative path, or macOS/var→/private/var) is discarded with no diagnostic.
crates/orca-tools/src/sandbox/mod.rs#L294-L301: canonicalize eachmetadata_writable_rootsentry first, then apply the safety filter to the canonical path.crates/orca-tools/src/sandbox/seatbelt.rs#L132-L132: applynormalize_path_for_seatbeltbeforeis_safe_metadata_writable_rootinworkspace_write_bash_command.crates/orca-tools/src/sandbox/seatbelt.rs#L179-L179: apply the same ordering inread_only_bash_command.Alternatively, document and enforce a canonical-path precondition at the API boundary so dropped grants surface as errors rather than silence.
🤖 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/orca-tools/src/sandbox/mod.rs` around lines 294 - 301, Normalize each metadata writable root before applying is_safe_metadata_writable_root, preserving only canonical safe paths. Update crates/orca-tools/src/sandbox/mod.rs:294-301, crates/orca-tools/src/sandbox/seatbelt.rs:132, and crates/orca-tools/src/sandbox/seatbelt.rs:179 so canonicalization or normalize_path_for_seatbelt precedes the safety filter at each site; retain the existing downstream grant generation behavior.
🧹 Nitpick comments (4)
crates/orca-runtime/src/runtime_permission.rs (1)
503-523: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCover symlink aliases and symlinked parents.
This test only exercises a path whose final component is
.agents. Add a case where an ordinary-looking alias or symlinked parent resolves into protected metadata, ensuring it is rejected from both writable-directory lists.🤖 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/orca-runtime/src/runtime_permission.rs` around lines 503 - 523, The test approved_symlink_metadata_root_is_not_made_writable currently covers only a symlink whose final component is .agents. Extend the test coverage with an ordinary-looking alias or symlinked-parent path that resolves to protected metadata, and assert that merge_permissions excludes it from both metadata_writable_directories() and additional_working_directories().tests/server_runtime_contract.rs (1)
1226-1254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen to prove selective filtering, not blanket wiping.
Only a single, reserved-source (
session-metadata) directory is seeded here, so this test can't distinguish "the runtime strips only reserved-source entries" from "the runtime wipes alladditional_working_directoriesat start regardless of source" — both would pass this assertion. Add a second, legitimately-sourced directory toconfig.additional_working_directoriesand assert it does survive persistence, while the forged one doesn't.🤖 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 `@tests/server_runtime_contract.rs` around lines 1226 - 1254, Update server_thread_start_cannot_forge_metadata_escalation by adding a second additional working directory with a legitimate, non-session-metadata source alongside the forged entry. After persistence, assert the reserved session-metadata directory is filtered out while the legitimate directory remains, preserving the existing metadata_writable_directories assertion.crates/orca-runtime/tests/runtime_surface_interaction.rs (1)
1599-1668: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSession-scope widening-rejection coverage was dropped, not replaced.
This test previously also verified that widening a Session-scope allow beyond the requested permissions is rejected; that step was removed here, and the new session test (
session_permission_allow_commits_settings_before_waking_and_survives_conflicting_retry) only grants the exact requested permissions at Session scope, never an invalid widened one. Since Session-scope grants are exactly the escalation surface this PR hardens, consider re-adding a widened-Session-scope rejection assertion (mirroring the existing Turn-scope one at Lines 1599-1630) unless it's verified to be covered elsewhere.🤖 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/orca-runtime/tests/runtime_surface_interaction.rs` around lines 1599 - 1668, Restore coverage for rejecting widened Session-scope permission grants in the permission interaction test. Add a Session-scoped Allow attempt with permissions exceeding the requested set, assert it returns an InvalidInput MutationReply and does not notify response_rx, then retain the existing exact-permission Session grant flow. Anchor the change near the current widened Turn-scope assertion and the session permission test.crates/orca-runtime/src/server/surface_adapter.rs (1)
918-926: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider encapsulating the source split so future consumers can't miss it.
Metadata grants now live inside
config.additional_working_directoriesdistinguished only bysource, and correctness depends on every reader filtering onSESSION_METADATA_DIRECTORY_SOURCE— this PR adds that filter in three places (settings_patcheshere,crates/orca-runtime/src/tool_router.rsline 289,crates/orca-runtime/src/runtime_bash.rsline 92). Any consumer that forgets silently grants ordinary write authority to.git/.agents. Two small helpers (e.g.ordinary_working_directories()/session_metadata_directories()) on the config would make the invariant hard to bypass.🤖 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/orca-runtime/src/server/surface_adapter.rs` around lines 918 - 926, The source-based split of additional working directories is exposed to every consumer and can be bypassed accidentally. Add config-level helpers for ordinary working directories and session metadata directories, then update settings_patches and the consumers in tool_router and runtime_bash to use those helpers instead of filtering SESSION_METADATA_DIRECTORY_SOURCE directly.
🤖 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/orca-runtime/src/runtime_host.rs`:
- Around line 18934-18949: Update prepare_surface_session_permission_grant and
its caller to carry the baseline thread revision from the snapshot used to build
the grant, then use that carried revision for
SurfaceEvent::SettingsPatch::Committed.previous_revision instead of re-reading
resident_surface.coordinator.state().snapshot().settings.thread_revision.
Preserve the existing grant settings and event behavior while keeping the
revision pair consistent.
- Around line 18891-18916: Update the public session-permission grant path in
the session_permission_grant construction to preserve and return the validation
error from prepare_surface_session_permission_grant as the same uncommitted
InvalidInput response used by the private path, rather than mapping all failures
to RuntimeUnavailable. Keep the successful grant flow unchanged.
- Around line 8064-8092: Update the ReplaceAdditionalWorkingDirectories branch
in the RuntimeSettingsPatch handler to reject or normalize entries whose source
equals SESSION_METADATA_DIRECTORY_SOURCE before copying them into
config.additional_working_directories. Ensure regular directory patches cannot
create session-metadata-provenance entries, while metadata roots remain
controlled exclusively by ReplaceMetadataWritableDirectories.
In `@crates/orca-runtime/src/runtime_surface/operation.rs`:
- Around line 1001-1002: Include metadata_writable_directories in the
replayability or authority fingerprint used for tool interactions, ensuring its
canonical directory values affect the computed digest and invalidate previously
bounded capability interactions when metadata write access changes. Update the
relevant replayable capsule, replayability digest, or authority payload
construction rather than only the field declaration on the operation
configuration.
In `@crates/orca-runtime/src/server/command_exec_sandbox.rs`:
- Around line 571-577: Update the test setup for
command_exec_sandbox_keeps_exact_metadata_profile_roots so
metadata-profile-workspace/.git exists before the profile roots are evaluated,
or adjust the profile gate to avoid requiring disk metadata for this test;
ensure the fixture root is still classified into
sandbox.metadata_writable_roots.
In `@crates/orca-tools/src/sandbox/bwrap.rs`:
- Around line 73-120: Update effective_read_only_roots and the Linux sandbox
launch flow so the effective root set is computed once per request and reused by
policy_requires_bwrap, landlock_command, and build_bwrap_argv instead of walking
metadata_protection_roots repeatedly. Precompute canonicalized
metadata_writable_roots once before traversal, and change
is_explicit_metadata_writable_root to accept that canonical grant slice rather
than the full LinuxSandboxPolicy.
In `@crates/orca-tools/src/sandbox/mod.rs`:
- Around line 23-32: Update the launch-gate flow around
is_safe_metadata_writable_root so each approved metadata-write path is
re-resolved and validated as an owned, non-symlink trusted source immediately
before constructing bubblewrap argv or applying Landlock rules. Reject the grant
if resolution fails or the canonical source no longer matches the approved path,
and ensure both backends use only that validated resolved source.
---
Outside diff comments:
In `@crates/orca-tools/src/sandbox/mod.rs`:
- Around line 294-301: Normalize each metadata writable root before applying
is_safe_metadata_writable_root, preserving only canonical safe paths. Update
crates/orca-tools/src/sandbox/mod.rs:294-301,
crates/orca-tools/src/sandbox/seatbelt.rs:132, and
crates/orca-tools/src/sandbox/seatbelt.rs:179 so canonicalization or
normalize_path_for_seatbelt precedes the safety filter at each site; retain the
existing downstream grant generation behavior.
---
Nitpick comments:
In `@crates/orca-runtime/src/runtime_permission.rs`:
- Around line 503-523: The test
approved_symlink_metadata_root_is_not_made_writable currently covers only a
symlink whose final component is .agents. Extend the test coverage with an
ordinary-looking alias or symlinked-parent path that resolves to protected
metadata, and assert that merge_permissions excludes it from both
metadata_writable_directories() and additional_working_directories().
In `@crates/orca-runtime/src/server/surface_adapter.rs`:
- Around line 918-926: The source-based split of additional working directories
is exposed to every consumer and can be bypassed accidentally. Add config-level
helpers for ordinary working directories and session metadata directories, then
update settings_patches and the consumers in tool_router and runtime_bash to use
those helpers instead of filtering SESSION_METADATA_DIRECTORY_SOURCE directly.
In `@crates/orca-runtime/tests/runtime_surface_interaction.rs`:
- Around line 1599-1668: Restore coverage for rejecting widened Session-scope
permission grants in the permission interaction test. Add a Session-scoped Allow
attempt with permissions exceeding the requested set, assert it returns an
InvalidInput MutationReply and does not notify response_rx, then retain the
existing exact-permission Session grant flow. Anchor the change near the current
widened Turn-scope assertion and the session permission test.
In `@tests/server_runtime_contract.rs`:
- Around line 1226-1254: Update
server_thread_start_cannot_forge_metadata_escalation by adding a second
additional working directory with a legitimate, non-session-metadata source
alongside the forged entry. After persistence, assert the reserved
session-metadata directory is filtered out while the legitimate directory
remains, preserving the existing metadata_writable_directories assertion.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 48d690c8-24c1-4a7d-9eb7-dd031f0b9a23
📒 Files selected for processing (30)
crates/orca-provider/src/lib.rscrates/orca-runtime/src/runtime_bash.rscrates/orca-runtime/src/runtime_host.rscrates/orca-runtime/src/runtime_permission.rscrates/orca-runtime/src/runtime_surface/commands.rscrates/orca-runtime/src/runtime_surface/commit.rscrates/orca-runtime/src/runtime_surface/operation.rscrates/orca-runtime/src/runtime_surface/reducer.rscrates/orca-runtime/src/server.rscrates/orca-runtime/src/server/command_exec_sandbox.rscrates/orca-runtime/src/server/processors/permission.rscrates/orca-runtime/src/server/surface_adapter.rscrates/orca-runtime/src/tool_router.rscrates/orca-runtime/tests/runtime_surface_attach.rscrates/orca-runtime/tests/runtime_surface_commit.rscrates/orca-runtime/tests/runtime_surface_interaction.rscrates/orca-runtime/tests/runtime_surface_reducer.rscrates/orca-tools/src/external.rscrates/orca-tools/src/sandbox/bwrap.rscrates/orca-tools/src/sandbox/linux.rscrates/orca-tools/src/sandbox/mod.rscrates/orca-tools/src/sandbox/seatbelt.rscrates/orca-tui/src/app.rsdocs/releases/v0.3.0.mdsite/public/sitemap.xmlsite/src/changelog/Changelog.tsxsite/src/shared.tstests/server_runtime_contract.rstests/session_server_contract.rstests/tui_pty_contract.rs
💤 Files with no reviewable changes (1)
- crates/orca-runtime/src/server/processors/permission.rs
| let mut events = Vec::new(); | ||
| if let Some((next_settings, _)) = session_permission_grant.as_ref() { | ||
| events.push(( | ||
| surface::SurfaceScope::Thread, | ||
| surface::SurfaceEvent::Settings(surface::SettingsPatch::Committed { | ||
| previous_revision: self | ||
| .resident_surface | ||
| .coordinator | ||
| .state() | ||
| .snapshot() | ||
| .settings | ||
| .thread_revision, | ||
| snapshot: next_settings.clone(), | ||
| }), | ||
| )); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Derive previous_revision from the snapshot the grant was built on.
next_settings.thread_revision is baseline + 1 from the snapshot read at Line 18747 / Line 18905, but previous_revision is re-read from the live coordinator state here. If any settings commit lands between those reads, the pair is inconsistent and the grant silently overwrites the concurrent change (its effective was cloned from the stale baseline). Carry the baseline revision out of prepare_surface_session_permission_grant and use it directly.
🤖 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/orca-runtime/src/runtime_host.rs` around lines 18934 - 18949, Update
prepare_surface_session_permission_grant and its caller to carry the baseline
thread revision from the snapshot used to build the grant, then use that carried
revision for SurfaceEvent::SettingsPatch::Committed.previous_revision instead of
re-reading
resident_surface.coordinator.state().snapshot().settings.thread_revision.
Preserve the existing grant settings and event behavior while keeping the
revision pair consistent.
3367a2a to
5e8f6e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/orca-runtime/tests/runtime_surface_interaction.rs (1)
1627-1633: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestore the Session-scope widening regression check.
native_permission_allow_cannot_widen_requested_profileonly rejects widenedPermissionGrantScope::Turnpermissions here, whilePermissionGrantScope::Sessionremains supported for permission responses. Add a Session-scope widening rejection, or a matching Session-scope subset assertion that Session permissions cannot exceed the requested profile.🤖 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/orca-runtime/tests/runtime_surface_interaction.rs` around lines 1627 - 1633, Extend native_permission_allow_cannot_widen_requested_profile to cover PermissionGrantScope::Session as well as Turn: add an assertion that a Session permission exceeding the requested profile is rejected with InvalidInput, or verify the equivalent Session subset constraint. Keep the existing response-channel and committed-value checks intact.crates/orca-tools/src/sandbox/linux.rs (1)
71-87: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFail-closed ordering is right; the message should tell the operator what to do.
Moving the
policy_requires_bwrapgate ahead of the Landlock fallback is the correct posture. Note the blast radius, though: on a Linux host withoutbwrap, any workspace-write policy whose cwd contains.gitnow overlaps a protected read-only root, so every sandboxed command exits 126 (which the new test at lines 576-606 pins). Since.gitin the workspace is the common case, this effectively requiresbubblewrapon Linux. Make the failure self-explanatory rather than making users bisect it.♻️ Actionable fail-closed message
if policy_requires_bwrap(&request) { - return fail_closed_command("this Linux sandbox policy requires bubblewrap"); + return fail_closed_command( + "this Linux sandbox policy requires bubblewrap (protected metadata under a writable root cannot be enforced by Landlock); install `bwrap` to run sandboxed commands", + ); }Confirm the release notes call out the new
bubblewraprequirement for Linux hosts.🤖 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/orca-tools/src/sandbox/linux.rs` around lines 71 - 87, Update the fail-closed message returned by policy_requires_bwrap in the Linux sandbox flow to explain that bubblewrap is required and must be installed on the host, including actionable installation guidance if an established project convention exists. Also update the release notes to call out the new bubblewrap requirement for Linux hosts, while leaving the backend ordering and exit behavior unchanged.
♻️ Duplicate comments (3)
crates/orca-runtime/src/runtime_host.rs (3)
18891-18916: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPermanent validation failures still map to
RuntimeUnavailable.Line 18912 collapses every
prepare_surface_session_permission_granterror into a transient-lookingRuntimeUnavailable, which clients retry. The private path at Line 18754 returns an uncommittedInvalidInputwith the message; mirror it here.🤖 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/orca-runtime/src/runtime_host.rs` around lines 18891 - 18916, Update the non-private session permission grant branch in the interaction handling flow to preserve permanent validation failures from prepare_surface_session_permission_grant instead of mapping all errors to RuntimeUnavailable. Mirror the private path’s uncommitted InvalidInput response, including the original error message, while retaining the existing successful grant behavior.
18934-18949: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
previous_revisionis re-read from live state instead of the grant baseline.
next_settings.thread_revisionisbaseline + 1from the snapshot read at Line 18747 / Line 18905, butprevious_revisionhere re-reads the coordinator. Any settings commit landing in between makes the pair inconsistent and silently overwrites the concurrent change. Carry the baseline revision out ofprepare_surface_session_permission_grant.🤖 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/orca-runtime/src/runtime_host.rs` around lines 18934 - 18949, Carry the baseline thread revision returned by prepare_surface_session_permission_grant through the session permission grant data, alongside next_settings. In the SurfaceEvent::Settings committed construction, set previous_revision from that captured baseline instead of re-reading resident_surface.coordinator.state().snapshot(); preserve next_settings as the committed snapshot.
8064-8092: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
ReplaceAdditionalWorkingDirectoriesstill copies caller-suppliedsourceverbatim.The new
retainpreserves pre-existing metadata entries, but Line 8072 copiesdirectory.sourcefrom the patch. A patch entry carryingsource == SESSION_METADATA_DIRECTORY_SOURCEbecomes a metadata write root (it is whatruntime_bash/snapshot derivation selects on) without passing theReplaceMetadataWritableDirectoriesgate at Line 23637. Filter or normalize that source here.🛡️ Proposed guard
config.additional_working_directories = directories .iter() + .filter(|directory| { + directory.source.as_str() + != crate::runtime_permission::SESSION_METADATA_DIRECTORY_SOURCE + }) .map(|directory| orca_core::config::AdditionalWorkingDirectory {🤖 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/orca-runtime/src/runtime_host.rs` around lines 8064 - 8092, Update the ReplaceAdditionalWorkingDirectories branch in the runtime settings patch handler so caller-supplied entries cannot retain or become SESSION_METADATA_DIRECTORY_SOURCE entries. Normalize their source to the ordinary additional-working-directory source or filter out entries using the reserved metadata source before collecting them, while preserving the separately retained existing metadata entries and the ReplaceMetadataWritableDirectories gate.
🧹 Nitpick comments (2)
crates/orca-runtime/tests/runtime_surface_interaction.rs (1)
3976-3981: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert that metadata grants stay out of ordinary writable roots.
This assertion verifies only
metadata_writable_directories. Add a check that the canonical metadata path is absent fromadditional_working_directories; otherwise a regression could grant the same path through the ordinary writable-root channel while this test still passes.🤖 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/orca-runtime/tests/runtime_surface_interaction.rs` around lines 3976 - 3981, Extend the test assertion around fresh_snapshot(&surface).settings.effective to verify that the canonical metadata path is absent from additional_working_directories, while retaining the existing metadata_writable_directories assertion.crates/orca-runtime/src/runtime_permission.rs (1)
429-437: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDropped metadata grants are silent — consider logging.
A protected metadata root that fails
is_safe_metadata_writable_root(symlink, non-canonical, missing) is discarded with no trace: it is not added tometadata_writable_directories, and theelse ifbranch is not reachable for it either. Failing closed is correct, but the caller gets anAllowresponse whose grant silently had no effect, so the sandbox will keep denying and re-prompting. Adebug/warnline naming the rejected root would make this diagnosable.Note this also contradicts the change description, which states non-safe protected metadata roots are "handled via the non-metadata working-directory path" — they are not.
🤖 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/orca-runtime/src/runtime_permission.rs` around lines 429 - 437, Update the protected-root handling around is_protected_metadata_root so roots rejected by is_safe_metadata_writable_root are explicitly logged with the rejected root and continue to fail closed. Do not route rejected protected metadata roots through additional_working_directories; preserve the existing insertion behavior for safe metadata roots and ordinary working directories.
🤖 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/orca-runtime/src/server.rs`:
- Around line 3721-3757: Update the later-agent-turn test around
execute_bash_with_shell_session to use the thread’s rehydrated configuration and
persisted permission overlay rather than server_config.run_config and a default
TurnPermissionOverlay. Exercise the actual subsequent-turn rehydration path, or
explicitly load the thread settings and grants before invoking runtime_bash,
while preserving the existing metadata write assertions.
In `@crates/orca-tools/src/sandbox/bwrap.rs`:
- Around line 440-458: Update
symlinked_metadata_does_not_treat_its_writable_target_as_an_explicit_grant so it
asserts a read-only bind for the .git link path, not its canonical target path,
and verifies the unrelated writable-root grant remains unchanged. Adjust the
argv lookup and assertions to reflect non-canonicalized symlinked metadata
handling.
- Around line 91-101: The metadata-root walk in effective_read_only_roots must
protect symlink paths without resolving their targets: when
entry.path_is_symlink() is true, push the entry path; otherwise retain
canonicalization for regular entries. Update
symlinked_metadata_does_not_treat_its_writable_target_as_an_explicit_grant to
verify the .git link itself is rebound read-only, and add coverage where it
targets a different granted writable root while that root retains its --bind.
In `@crates/orca-tools/src/sandbox/mod.rs`:
- Around line 23-32: Update is_safe_metadata_writable_root to stop requiring the
full path’s canonical form to equal the original path, so valid metadata roots
under symlinked prefixes remain accepted. Retain the protected-root and
final-component symlink checks, and instead validate the parent path via
canonicalization (or ensure callers provide canonical metadata roots) before
accepting the root.
In `@tests/server_runtime_contract.rs`:
- Around line 1209-1218: Broaden the assertion in the persisted
working-directory check so `git_dir` is rejected regardless of its `source`
value, rather than only rejecting the exact `"session-metadata"` pair. Preserve
the existing intent and message that reserved metadata directories must not be
persisted as ordinary directory updates.
In `@tests/session_server_contract.rs`:
- Around line 9165-9196: Update
server_mode_session_metadata_grant_is_committed_before_later_agent_turn_uses_it
to require sandbox_seatbelt_available() in addition to the macOS cfg, matching
the sandbox-dependent tests and ensuring Seatbelt enforcement is active. Also
replace the hand-built JSON strings for the turn/start and permission/respond
payloads with json!/serde_json serialization so Path::display() values are
escaped safely.
In `@tests/tui_pty_contract.rs`:
- Around line 90-96: Update the mock provider loop in the provider branch around
the tool-call generation logic to emit the two tool calls only when
!has_tool_results. Preserve the generic completion response for the subsequent
turn, and keep the receive_until assertion in tests/tui_pty_contract.rs after
the permission and Bash results.
---
Outside diff comments:
In `@crates/orca-runtime/tests/runtime_surface_interaction.rs`:
- Around line 1627-1633: Extend
native_permission_allow_cannot_widen_requested_profile to cover
PermissionGrantScope::Session as well as Turn: add an assertion that a Session
permission exceeding the requested profile is rejected with InvalidInput, or
verify the equivalent Session subset constraint. Keep the existing
response-channel and committed-value checks intact.
In `@crates/orca-tools/src/sandbox/linux.rs`:
- Around line 71-87: Update the fail-closed message returned by
policy_requires_bwrap in the Linux sandbox flow to explain that bubblewrap is
required and must be installed on the host, including actionable installation
guidance if an established project convention exists. Also update the release
notes to call out the new bubblewrap requirement for Linux hosts, while leaving
the backend ordering and exit behavior unchanged.
---
Duplicate comments:
In `@crates/orca-runtime/src/runtime_host.rs`:
- Around line 18891-18916: Update the non-private session permission grant
branch in the interaction handling flow to preserve permanent validation
failures from prepare_surface_session_permission_grant instead of mapping all
errors to RuntimeUnavailable. Mirror the private path’s uncommitted InvalidInput
response, including the original error message, while retaining the existing
successful grant behavior.
- Around line 18934-18949: Carry the baseline thread revision returned by
prepare_surface_session_permission_grant through the session permission grant
data, alongside next_settings. In the SurfaceEvent::Settings committed
construction, set previous_revision from that captured baseline instead of
re-reading resident_surface.coordinator.state().snapshot(); preserve
next_settings as the committed snapshot.
- Around line 8064-8092: Update the ReplaceAdditionalWorkingDirectories branch
in the runtime settings patch handler so caller-supplied entries cannot retain
or become SESSION_METADATA_DIRECTORY_SOURCE entries. Normalize their source to
the ordinary additional-working-directory source or filter out entries using the
reserved metadata source before collecting them, while preserving the separately
retained existing metadata entries and the ReplaceMetadataWritableDirectories
gate.
---
Nitpick comments:
In `@crates/orca-runtime/src/runtime_permission.rs`:
- Around line 429-437: Update the protected-root handling around
is_protected_metadata_root so roots rejected by is_safe_metadata_writable_root
are explicitly logged with the rejected root and continue to fail closed. Do not
route rejected protected metadata roots through additional_working_directories;
preserve the existing insertion behavior for safe metadata roots and ordinary
working directories.
In `@crates/orca-runtime/tests/runtime_surface_interaction.rs`:
- Around line 3976-3981: Extend the test assertion around
fresh_snapshot(&surface).settings.effective to verify that the canonical
metadata path is absent from additional_working_directories, while retaining the
existing metadata_writable_directories assertion.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5df0540b-6aff-4a90-9a09-1596a46cf9c4
📒 Files selected for processing (32)
crates/orca-provider/src/lib.rscrates/orca-runtime/src/runtime_bash.rscrates/orca-runtime/src/runtime_host.rscrates/orca-runtime/src/runtime_permission.rscrates/orca-runtime/src/runtime_surface/commands.rscrates/orca-runtime/src/runtime_surface/commit.rscrates/orca-runtime/src/runtime_surface/operation.rscrates/orca-runtime/src/runtime_surface/reducer.rscrates/orca-runtime/src/server.rscrates/orca-runtime/src/server/command_exec_sandbox.rscrates/orca-runtime/src/server/processors/permission.rscrates/orca-runtime/src/server/surface_adapter.rscrates/orca-runtime/src/tool_router.rscrates/orca-runtime/tests/runtime_surface_attach.rscrates/orca-runtime/tests/runtime_surface_commit.rscrates/orca-runtime/tests/runtime_surface_interaction.rscrates/orca-runtime/tests/runtime_surface_reducer.rscrates/orca-tools/src/external.rscrates/orca-tools/src/sandbox/bwrap.rscrates/orca-tools/src/sandbox/linux.rscrates/orca-tools/src/sandbox/mod.rscrates/orca-tools/src/sandbox/seatbelt.rscrates/orca-tui/src/app.rsdocs/releases/v0.3.0.mddocs/superpowers/specs/2026-07-28-native-windows-platform-foundation.manifest.jsonsite/public/sitemap.xmlsite/src/changelog/Changelog.tsxsite/src/shared.tstests/server_runtime_contract.rstests/session_server_contract.rstests/subagent_contract.rstests/tui_pty_contract.rs
💤 Files with no reviewable changes (1)
- crates/orca-runtime/src/server/processors/permission.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- site/src/shared.ts
- site/src/changelog/Changelog.tsx
- crates/orca-runtime/src/runtime_surface/commands.rs
- crates/orca-runtime/tests/runtime_surface_reducer.rs
- docs/releases/v0.3.0.md
|
|
||
| let agent_target = repo.join(".git").join("agent-next-turn.lock"); | ||
| let agent_request = orca_core::tool_types::ToolRequest { | ||
| id: "agent-session-metadata".to_string(), | ||
| name: orca_core::tool_types::ToolName::Bash, | ||
| action: orca_core::approval_types::ActionKind::Write, | ||
| target: Some(format!("printf agent > {}", agent_target.display())), | ||
| raw_arguments: None, | ||
| }; | ||
| let task_registry = crate::tasks::TaskRegistry::new(thread_id.clone()); | ||
| let mut permission_overlay = | ||
| crate::runtime_permission::TurnPermissionOverlay::default(); | ||
| let agent_result = crate::runtime_bash::execute_bash_with_shell_session( | ||
| crate::runtime_bash::RuntimeBashInvocationContext { | ||
| config: Some(&server_config.run_config), | ||
| request: &agent_request, | ||
| cwd: &repo, | ||
| additional_roots: &[], | ||
| output_truncation: orca_core::tool_types::ToolOutputTruncation::default(), | ||
| shell_timeout_secs: 5, | ||
| task_registry: &task_registry, | ||
| cancel: None, | ||
| permission_handler: None, | ||
| permission_overlay: &mut permission_overlay, | ||
| output_handler: None, | ||
| }, | ||
| ); | ||
|
|
||
| assert_eq!( | ||
| agent_result.status, | ||
| orca_core::tool_types::ToolStatus::Completed, | ||
| "a later agent turn should rehydrate the session metadata grant: {agent_result:?}" | ||
| ); | ||
| assert_eq!( | ||
| std::fs::read_to_string(agent_target).expect("agent session metadata write"), | ||
| "agent" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Exercise the rehydrated thread configuration, not the original server config.
Line 3733 passes server_config.run_config and Line 3732 uses a default overlay, so the persisted .git grant is never supplied to runtime_bash. Its metadata-root logic will therefore deny this write after hardening; this test neither validates nor simulates a later agent turn. Execute through the thread’s subsequent-turn path (or explicitly load its rehydrated settings) instead.
🤖 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/orca-runtime/src/server.rs` around lines 3721 - 3757, Update the
later-agent-turn test around execute_bash_with_shell_session to use the thread’s
rehydrated configuration and persisted permission overlay rather than
server_config.run_config and a default TurnPermissionOverlay. Exercise the
actual subsequent-turn rehydration path, or explicitly load the thread settings
and grants before invoking runtime_bash, while preserving the existing metadata
write assertions.
| Ok(entry) if crate::sandbox::is_protected_metadata_root(entry.path()) => { | ||
| let canonical = entry | ||
| .path() | ||
| .canonicalize() | ||
| .unwrap_or_else(|_| entry.path().to_path_buf()); | ||
| if !is_explicit_metadata_writable_root(policy, entry.path(), &canonical) { | ||
| push_unique(&mut roots, canonical); | ||
| } | ||
| if entry.file_type().is_dir() { | ||
| entries.skip_current_dir(); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Resolving through a metadata symlink is one root cause spanning the walk and its test. effective_read_only_roots canonicalizes any entry whose file name matches a protected metadata dir, so a symlink named .git/.agents/.codex inside a writable root resolves to its target and that target is re-bound --ro-bind after the writable binds — silently revoking write access to whatever the link points at. The walk already sets follow_links(false), so the link path itself is what should be protected.
crates/orca-tools/src/sandbox/bwrap.rs#L91-L101: whenentry.path_is_symlink(), push the entry path instead of its canonicalized target.crates/orca-tools/src/sandbox/bwrap.rs#L440-L458: updatesymlinked_metadata_does_not_treat_its_writable_target_as_an_explicit_grantto assert the.gitlink path is re-bound read-only, and add a case where the link targets a different granted writable root to prove that root keeps its--bind.
📍 Affects 1 file
crates/orca-tools/src/sandbox/bwrap.rs#L91-L101(this comment)crates/orca-tools/src/sandbox/bwrap.rs#L440-L458
🤖 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/orca-tools/src/sandbox/bwrap.rs` around lines 91 - 101, The
metadata-root walk in effective_read_only_roots must protect symlink paths
without resolving their targets: when entry.path_is_symlink() is true, push the
entry path; otherwise retain canonicalization for regular entries. Update
symlinked_metadata_does_not_treat_its_writable_target_as_an_explicit_grant to
verify the .git link itself is rebound read-only, and add coverage where it
targets a different granted writable root while that root retains its --bind.
| #[cfg(unix)] | ||
| #[test] | ||
| fn symlinked_metadata_does_not_treat_its_writable_target_as_an_explicit_grant() { | ||
| let external = tempfile::tempdir().unwrap(); | ||
| let metadata = external.path().join(".git"); | ||
| std::os::unix::fs::symlink(external.path(), &metadata).unwrap(); | ||
| let mut policy = base_policy(); | ||
| let target = external.path().canonicalize().unwrap(); | ||
| policy.writable_roots = vec![target.clone()]; | ||
| policy.metadata_protection_roots = policy.writable_roots.clone(); | ||
|
|
||
| let argv = build_bwrap_argv(&policy, "true"); | ||
| let target = path_arg(&target); | ||
| let read_only_position = argv | ||
| .windows(3) | ||
| .position(|args| args == ["--ro-bind", target.as_str(), target.as_str()]); | ||
|
|
||
| assert!(read_only_position.is_some()); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
This test locks in resolving through the metadata symlink.
The scenario is a .git symlink whose target is the writable root, so asserting --ro-bind <target> <target> looks benign. But the assertion encodes the general behavior that a symlink named .git re-binds its canonical target read-only, which is the escape flagged on lines 91-101 (a .git link pointing at an unrelated granted root revokes that root). Once the walk stops canonicalizing symlinked metadata entries, this should assert the link path is re-bound read-only and that the unrelated target grant is untouched.
🤖 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/orca-tools/src/sandbox/bwrap.rs` around lines 440 - 458, Update
symlinked_metadata_does_not_treat_its_writable_target_as_an_explicit_grant so it
asserts a read-only bind for the .git link path, not its canonical target path,
and verifies the unrelated writable-root grant remains unchanged. Adjust the
argv lookup and assertions to reflect non-canonicalized symlinked metadata
handling.
| assert!( | ||
| persisted | ||
| !persisted | ||
| .meta | ||
| .additional_working_directories | ||
| .iter() | ||
| .any(|directory| { | ||
| directory.path == git_dir && directory.source == "session-metadata" | ||
| }) | ||
| }), | ||
| "reserved metadata sources must be ignored by ordinary directory updates" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assertion is narrower than its message.
It only rejects the pair (git_dir, "session-metadata"), so a regression that rewrites the reserved source (e.g. to "session") while still persisting .git as an ordinary working directory would pass. The message claims reserved metadata sources are "ignored".
💚 Assert the directory is not persisted at all
assert!(
!persisted
.meta
.additional_working_directories
.iter()
- .any(|directory| {
- directory.path == git_dir && directory.source == "session-metadata"
- }),
+ .any(|directory| directory.path == git_dir),
"reserved metadata sources must be ignored by ordinary directory updates"
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert!( | |
| persisted | |
| !persisted | |
| .meta | |
| .additional_working_directories | |
| .iter() | |
| .any(|directory| { | |
| directory.path == git_dir && directory.source == "session-metadata" | |
| }) | |
| }), | |
| "reserved metadata sources must be ignored by ordinary directory updates" | |
| ); | |
| assert!( | |
| !persisted | |
| .meta | |
| .additional_working_directories | |
| .iter() | |
| .any(|directory| directory.path == git_dir), | |
| "reserved metadata sources must be ignored by ordinary directory updates" | |
| ); |
🤖 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 `@tests/server_runtime_contract.rs` around lines 1209 - 1218, Broaden the
assertion in the persisted working-directory check so `git_dir` is rejected
regardless of its `source` value, rather than only rejecting the exact
`"session-metadata"` pair. Preserve the existing intent and message that
reserved metadata directories must not be persisted as ordinary directory
updates.
| #[cfg(target_os = "macos")] | ||
| #[test] | ||
| fn server_mode_session_metadata_grant_is_committed_before_later_agent_turn_uses_it() { | ||
| let parent = sandbox_test_parent("orca-request-permissions-session-metadata-"); | ||
| let workspace = parent.path().join("workspace"); | ||
| let home = parent.path().join("home"); | ||
| let metadata = workspace.join(".git"); | ||
| std::fs::create_dir_all(&metadata).expect("create metadata"); | ||
| std::fs::create_dir_all(&home).expect("create home"); | ||
| std::fs::write( | ||
| home.join("config.toml"), | ||
| "mode = \"suggest\"\n[[permissions.rules]]\ntool = \"bash\"\npattern = \"**\"\ndecision = \"allow\"\n", | ||
| ) | ||
| .expect("write config"); | ||
| let first_output = metadata.join("first.lock"); | ||
| let second_output = metadata.join("second.lock"); | ||
|
|
||
| let mut child = orca_command() | ||
| .args([ | ||
| "--mode", | ||
| "server", | ||
| "--provider", | ||
| "mock", | ||
| "--cwd", | ||
| workspace.to_str().unwrap(), | ||
| ]) | ||
| .env("ORCA_HOME", &home) | ||
| .stdin(Stdio::piped()) | ||
| .stdout(Stdio::piped()) | ||
| .stderr(Stdio::piped()) | ||
| .spawn() | ||
| .expect("spawn orca server"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Gate this on Seatbelt availability like its siblings.
Every other sandbox-dependent test in this file routes through sandbox_seatbelt_available() (which asserts /usr/bin/sandbox-exec actually works on macOS). This one relies on #[cfg(target_os = "macos")] alone, so if sandbox-exec were unavailable the writes would succeed for the wrong reason and the test would still pass while proving nothing about metadata grant enforcement.
💚 Assert the sandbox is actually enforcing
#[cfg(target_os = "macos")]
#[test]
fn server_mode_session_metadata_grant_is_committed_before_later_agent_turn_uses_it() {
+ if !sandbox_seatbelt_available() {
+ return;
+ }
let parent = sandbox_test_parent("orca-request-permissions-session-metadata-");Minor, same segment: the turn/start and permission/respond payloads at lines 9216-9236 interpolate Path::display() straight into a JSON string literal, whereas neighbouring tests build them with json!/serde_json::to_string. Switching to json! keeps it robust if a temp path ever contains a quote or backslash.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[cfg(target_os = "macos")] | |
| #[test] | |
| fn server_mode_session_metadata_grant_is_committed_before_later_agent_turn_uses_it() { | |
| let parent = sandbox_test_parent("orca-request-permissions-session-metadata-"); | |
| let workspace = parent.path().join("workspace"); | |
| let home = parent.path().join("home"); | |
| let metadata = workspace.join(".git"); | |
| std::fs::create_dir_all(&metadata).expect("create metadata"); | |
| std::fs::create_dir_all(&home).expect("create home"); | |
| std::fs::write( | |
| home.join("config.toml"), | |
| "mode = \"suggest\"\n[[permissions.rules]]\ntool = \"bash\"\npattern = \"**\"\ndecision = \"allow\"\n", | |
| ) | |
| .expect("write config"); | |
| let first_output = metadata.join("first.lock"); | |
| let second_output = metadata.join("second.lock"); | |
| let mut child = orca_command() | |
| .args([ | |
| "--mode", | |
| "server", | |
| "--provider", | |
| "mock", | |
| "--cwd", | |
| workspace.to_str().unwrap(), | |
| ]) | |
| .env("ORCA_HOME", &home) | |
| .stdin(Stdio::piped()) | |
| .stdout(Stdio::piped()) | |
| .stderr(Stdio::piped()) | |
| .spawn() | |
| .expect("spawn orca server"); | |
| #[cfg(target_os = "macos")] | |
| #[test] | |
| fn server_mode_session_metadata_grant_is_committed_before_later_agent_turn_uses_it() { | |
| if !sandbox_seatbelt_available() { | |
| return; | |
| } | |
| let parent = sandbox_test_parent("orca-request-permissions-session-metadata-"); | |
| let workspace = parent.path().join("workspace"); | |
| let home = parent.path().join("home"); | |
| let metadata = workspace.join(".git"); | |
| std::fs::create_dir_all(&metadata).expect("create metadata"); | |
| std::fs::create_dir_all(&home).expect("create home"); | |
| std::fs::write( | |
| home.join("config.toml"), | |
| "mode = \"suggest\"\n[[permissions.rules]]\ntool = \"bash\"\npattern = \"**\"\ndecision = \"allow\"\n", | |
| ) | |
| .expect("write config"); | |
| let first_output = metadata.join("first.lock"); | |
| let second_output = metadata.join("second.lock"); | |
| let mut child = orca_command() | |
| .args([ | |
| "--mode", | |
| "server", | |
| "--provider", | |
| "mock", | |
| "--cwd", | |
| workspace.to_str().unwrap(), | |
| ]) | |
| .env("ORCA_HOME", &home) | |
| .stdin(Stdio::piped()) | |
| .stdout(Stdio::piped()) | |
| .stderr(Stdio::piped()) | |
| .spawn() | |
| .expect("spawn orca server"); |
🤖 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 `@tests/session_server_contract.rs` around lines 9165 - 9196, Update
server_mode_session_metadata_grant_is_committed_before_later_agent_turn_uses_it
to require sandbox_seatbelt_available() in addition to the macOS cfg, matching
the sandbox-dependent tests and ensuring Seatbelt enforcement is active. Also
replace the hand-built JSON strings for the turn/start and permission/respond
payloads with json!/serde_json serialization so Path::display() values are
escaped safely.
5d48160 to
52aed80
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
crates/orca-runtime/src/runtime_host.rs (3)
18891-18916: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidation failures on the public winner path still collapse into
RuntimeUnavailable.Line 18912 maps every
prepare_surface_session_permission_granterror to a transient-lookingRuntimeUnavailable, so clients retry a permanently invalid grant. The private path at Line 18754 already returns an uncommittedInvalidInputwith the message; mirror it here.🤖 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/orca-runtime/src/runtime_host.rs` around lines 18891 - 18916, The public winner path in the session permission grant handling should not map prepare_surface_session_permission_grant validation errors to RuntimeUnavailable. Update the map_err branch around prepare_surface_session_permission_grant to return the same uncommitted InvalidInput error, including the validation message, already used by the private path.
18934-18949: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
previous_revisionis re-read from live coordinator state.
next_settings.thread_revisionisbaseline + 1from the snapshot read at Line 18747 / Line 18905, but Line 18939 re-reads the current revision. Any settings commit landing in between makes the pair inconsistent and silently overwrites the concurrent change. Carry the baseline revision out ofprepare_surface_session_permission_grant.🤖 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/orca-runtime/src/runtime_host.rs` around lines 18934 - 18949, Update prepare_surface_session_permission_grant and its caller to carry the baseline thread revision alongside next_settings, then use that captured revision for SettingsPatch::Committed.previous_revision instead of re-reading live coordinator state through resident_surface. Keep next_settings.thread_revision paired with the same snapshot baseline.
8064-8092: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPatch-supplied
sourceis still copied verbatim at Line 8072.The retain at Line 8065 preserves existing session-metadata entries, but a client-supplied
ReplaceAdditionalWorkingDirectoriesentry whosesourceis"session-metadata"still lands inconfig.additional_working_directorieswith metadata provenance, whileReplaceMetadataWritableDirectoriesis gated asUnauthorizedat Line 23637. Normalize or reject that source here.♻️ Proposed normalization
config.additional_working_directories = directories .iter() + .filter(|directory| { + directory.source.as_str() + != crate::runtime_permission::SESSION_METADATA_DIRECTORY_SOURCE + }) .map(|directory| orca_core::config::AdditionalWorkingDirectory {🤖 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/orca-runtime/src/runtime_host.rs` around lines 8064 - 8092, Update the ReplaceAdditionalWorkingDirectories branch of the RuntimeSettingsPatch handler to prevent client-supplied entries with source SESSION_METADATA_DIRECTORY_SOURCE from being treated as metadata-provenance directories. Normalize those entries to a non-metadata source or reject them, while preserving the existing session-metadata entries retained from configuration and keeping settings.additional_working_directories synchronized.tests/server_runtime_contract.rs (1)
1210-1217: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssertion is still narrower than its failure message (past review comment not applied).
This is the same check a previous review flagged: it only rejects the exact pair
(git_dir, source == "session-metadata"), so a regression that persists.gitunder a different source string (e.g."session") would still pass, despite the message claiming reserved metadata sources are "ignored". The code is unchanged from what was previously flagged.💚 Assert the directory is not persisted at all
assert!( !persisted .meta .additional_working_directories .iter() - .any(|directory| { - directory.path == git_dir && directory.source == "session-metadata" - }), + .any(|directory| directory.path == git_dir), "reserved metadata sources must be ignored by ordinary directory updates" );🤖 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 `@tests/server_runtime_contract.rs` around lines 1210 - 1217, Update the assertion over persisted.meta.additional_working_directories to reject any entry whose path equals git_dir, regardless of its source value. Remove the source == "session-metadata" restriction while preserving the existing assertion message.
🤖 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/orca-runtime/src/runtime_host.rs`:
- Around line 10298-10306: Update the metadata-root checks in the session
metadata grant handling to classify the canonicalized path consistently: pass
canonical.as_path() to both is_protected_metadata_root and
is_safe_metadata_writable_root, while retaining the existing deduplication and
error behavior.
In `@crates/orca-runtime/src/runtime_permission.rs`:
- Around line 503-523: Update the
approved_symlink_metadata_root_is_not_made_writable test fixture to canonicalize
the tempfile parent before constructing target and metadata_link paths, ensuring
the parent path satisfies is_safe_metadata_writable_root’s canonical-path
requirement. Keep the symlink setup and empty-permissions assertions unchanged
so the test specifically validates rejection of the symlink.
In `@crates/orca-runtime/src/runtime_surface/commit.rs`:
- Around line 5190-5237: Strengthen
actor_generation_permission_resolution_authorized by extracting and validating
the committed settings and resolved interaction payload, requiring a
PermissionRequest, matching receipt.kind to the interaction kind, and confirming
the settings match the resolved request and expected state. Mirror the
interaction-state correlation checks in
provider_background_interaction_resolution_authorized, and add a covering test
for mismatched or non-permission resolutions, including JSONL recovery behavior.
---
Duplicate comments:
In `@crates/orca-runtime/src/runtime_host.rs`:
- Around line 18891-18916: The public winner path in the session permission
grant handling should not map prepare_surface_session_permission_grant
validation errors to RuntimeUnavailable. Update the map_err branch around
prepare_surface_session_permission_grant to return the same uncommitted
InvalidInput error, including the validation message, already used by the
private path.
- Around line 18934-18949: Update prepare_surface_session_permission_grant and
its caller to carry the baseline thread revision alongside next_settings, then
use that captured revision for SettingsPatch::Committed.previous_revision
instead of re-reading live coordinator state through resident_surface. Keep
next_settings.thread_revision paired with the same snapshot baseline.
- Around line 8064-8092: Update the ReplaceAdditionalWorkingDirectories branch
of the RuntimeSettingsPatch handler to prevent client-supplied entries with
source SESSION_METADATA_DIRECTORY_SOURCE from being treated as
metadata-provenance directories. Normalize those entries to a non-metadata
source or reject them, while preserving the existing session-metadata entries
retained from configuration and keeping settings.additional_working_directories
synchronized.
In `@tests/server_runtime_contract.rs`:
- Around line 1210-1217: Update the assertion over
persisted.meta.additional_working_directories to reject any entry whose path
equals git_dir, regardless of its source value. Remove the source ==
"session-metadata" restriction while preserving the existing assertion message.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4375b19b-b114-4016-bc4f-dd41f69251b0
📒 Files selected for processing (32)
crates/orca-provider/src/lib.rscrates/orca-runtime/src/runtime_bash.rscrates/orca-runtime/src/runtime_host.rscrates/orca-runtime/src/runtime_permission.rscrates/orca-runtime/src/runtime_surface/commands.rscrates/orca-runtime/src/runtime_surface/commit.rscrates/orca-runtime/src/runtime_surface/operation.rscrates/orca-runtime/src/runtime_surface/reducer.rscrates/orca-runtime/src/server.rscrates/orca-runtime/src/server/command_exec_sandbox.rscrates/orca-runtime/src/server/processors/permission.rscrates/orca-runtime/src/server/surface_adapter.rscrates/orca-runtime/src/tool_router.rscrates/orca-runtime/tests/runtime_surface_attach.rscrates/orca-runtime/tests/runtime_surface_commit.rscrates/orca-runtime/tests/runtime_surface_interaction.rscrates/orca-runtime/tests/runtime_surface_reducer.rscrates/orca-tools/src/external.rscrates/orca-tools/src/sandbox/bwrap.rscrates/orca-tools/src/sandbox/linux.rscrates/orca-tools/src/sandbox/mod.rscrates/orca-tools/src/sandbox/seatbelt.rscrates/orca-tui/src/app.rsdocs/releases/v0.3.0.mddocs/superpowers/specs/2026-07-28-native-windows-platform-foundation.manifest.jsonsite/public/sitemap.xmlsite/src/changelog/Changelog.tsxsite/src/shared.tstests/server_runtime_contract.rstests/session_server_contract.rstests/subagent_contract.rstests/tui_pty_contract.rs
💤 Files with no reviewable changes (1)
- crates/orca-runtime/src/server/processors/permission.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- site/src/shared.ts
- crates/orca-runtime/src/runtime_surface/reducer.rs
- site/src/changelog/Changelog.tsx
- docs/releases/v0.3.0.md
| #[cfg(unix)] | ||
| #[test] | ||
| fn approved_symlink_metadata_root_is_not_made_writable() { | ||
| let parent = tempfile::tempdir().unwrap(); | ||
| let target = parent.path().join("outside"); | ||
| let metadata_link = parent.path().join(".agents"); | ||
| std::fs::create_dir(&target).unwrap(); | ||
| std::os::unix::fs::symlink(&target, &metadata_link).unwrap(); | ||
| let mut overlay = TurnPermissionOverlay::default(); | ||
|
|
||
| overlay.merge_permissions(&RequestPermissionProfile { | ||
| file_system: Some(RequestFileSystemPermissions { | ||
| write: Some(vec![metadata_link]), | ||
| ..Default::default() | ||
| }), | ||
| ..Default::default() | ||
| }); | ||
|
|
||
| assert!(overlay.metadata_writable_directories().is_empty()); | ||
| assert!(overlay.additional_working_directories().is_empty()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Symlink test can pass for the wrong reason on macOS.
tempfile::tempdir() lives under /var/folders/..., whose canonical form is /private/var/.... is_safe_metadata_writable_root also requires path.canonicalize() == path, so this grant would be rejected on macOS even if the symlink check were removed. Anchoring the fixture in a canonicalized parent makes the assertion actually prove the symlink rule.
💚 Canonicalize the fixture parent
- let parent = tempfile::tempdir().unwrap();
- let target = parent.path().join("outside");
- let metadata_link = parent.path().join(".agents");
+ let parent = tempfile::tempdir().unwrap();
+ let parent = parent.path().canonicalize().unwrap();
+ let target = parent.join("outside");
+ let metadata_link = parent.join(".agents");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[cfg(unix)] | |
| #[test] | |
| fn approved_symlink_metadata_root_is_not_made_writable() { | |
| let parent = tempfile::tempdir().unwrap(); | |
| let target = parent.path().join("outside"); | |
| let metadata_link = parent.path().join(".agents"); | |
| std::fs::create_dir(&target).unwrap(); | |
| std::os::unix::fs::symlink(&target, &metadata_link).unwrap(); | |
| let mut overlay = TurnPermissionOverlay::default(); | |
| overlay.merge_permissions(&RequestPermissionProfile { | |
| file_system: Some(RequestFileSystemPermissions { | |
| write: Some(vec![metadata_link]), | |
| ..Default::default() | |
| }), | |
| ..Default::default() | |
| }); | |
| assert!(overlay.metadata_writable_directories().is_empty()); | |
| assert!(overlay.additional_working_directories().is_empty()); | |
| } | |
| #[cfg(unix)] | |
| #[test] | |
| fn approved_symlink_metadata_root_is_not_made_writable() { | |
| let parent = tempfile::tempdir().unwrap(); | |
| let parent = parent.path().canonicalize().unwrap(); | |
| let target = parent.join("outside"); | |
| let metadata_link = parent.join(".agents"); | |
| std::fs::create_dir(&target).unwrap(); | |
| std::os::unix::fs::symlink(&target, &metadata_link).unwrap(); | |
| let mut overlay = TurnPermissionOverlay::default(); | |
| overlay.merge_permissions(&RequestPermissionProfile { | |
| file_system: Some(RequestFileSystemPermissions { | |
| write: Some(vec![metadata_link]), | |
| ..Default::default() | |
| }), | |
| ..Default::default() | |
| }); | |
| assert!(overlay.metadata_writable_directories().is_empty()); | |
| assert!(overlay.additional_working_directories().is_empty()); | |
| } |
🤖 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/orca-runtime/src/runtime_permission.rs` around lines 503 - 523, Update
the approved_symlink_metadata_root_is_not_made_writable test fixture to
canonicalize the tempfile parent before constructing target and metadata_link
paths, ensuring the parent path satisfies is_safe_metadata_writable_root’s
canonical-path requirement. Keep the symlink setup and empty-permissions
assertions unchanged so the test specifically validates rejection of the
symlink.
| fn actor_generation_permission_resolution_authorized( | ||
| issued_permits: &[SurfacePublisherPermit], | ||
| actor_permit: &SurfacePublisherPermit, | ||
| generation_permit: &SurfacePublisherPermit, | ||
| batch: &SurfaceCommitBatch, | ||
| owner_epoch: ThreadOwnerEpoch, | ||
| ) -> bool { | ||
| if !issued_permits.contains(actor_permit) || !issued_permits.contains(generation_permit) { | ||
| return false; | ||
| } | ||
| let ( | ||
| SurfacePublisherPermit::ActorControl { | ||
| thread_id, | ||
| owner_epoch: actor_owner_epoch, | ||
| .. | ||
| }, | ||
| SurfacePublisherPermit::Generation { fence, .. }, | ||
| ) = (actor_permit, generation_permit) | ||
| else { | ||
| return false; | ||
| }; | ||
| if *actor_owner_epoch != owner_epoch | ||
| || thread_id != &fence.thread_id | ||
| || thread_id != &batch.cursor_before.thread_id | ||
| || thread_id != &batch.cursor_after.thread_id | ||
| { | ||
| return false; | ||
| } | ||
| let [settings, interaction] = batch.events.as_slice() else { | ||
| return false; | ||
| }; | ||
| let settings_valid = matches!( | ||
| (&settings.scope, &settings.event), | ||
| ( | ||
| SurfaceScope::Thread, | ||
| super::SurfaceEvent::Settings(super::SettingsPatch::Committed { .. }) | ||
| ) | ||
| ); | ||
| let interaction_valid = matches!( | ||
| (&interaction.scope, &interaction.event), | ||
| ( | ||
| SurfaceScope::Generation { fence: scope }, | ||
| super::SurfaceEvent::Interaction(super::InteractionPatch::Resolved { .. }) | ||
| ) if scope == fence | ||
| ); | ||
| settings_valid && interaction_valid | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "== SettingsPatch definition =="
rg -n -B2 -A20 'enum SettingsPatch' crates/orca-runtime/src -g '*.rs'
echo "== InteractionPatch::Resolved definition =="
rg -n -B5 -A15 'Resolved \{' crates/orca-runtime/src/runtime_surface -g '*.rs'
echo "== SurfaceInteractionKind variants =="
rg -n -A15 'enum SurfaceInteractionKind' crates/orca-runtime/src -g '*.rs'
echo "== callers of commit_actor_generation_permission_resolution_batch =="
rg -n -B5 -A40 'commit_actor_generation_permission_resolution_batch' crates/orca-runtime/src -g '*.rs'
echo "== reducer handling of SettingsPatch::Committed =="
rg -n -B5 -A40 'SettingsPatch::Committed' crates/orca-runtime/src/runtime_surface -g '*.rs'Repository: echoVic/orca-agent
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== commit.rs around target predicate =="
sed -n '5160,5255p' crates/orca-runtime/src/runtime_surface/commit.rs
echo
echo "== actor_generation_permission_resolution_batch and related functions =="
rg -n -B10 -A65 'actor_generation_permission_resolution_batch|issue_exact_recovered_authority|batch_with_authority' crates/orca-runtime/src/runtime_surface/commit.rs | sed -n '1,260p'
echo
echo "== tests named like actor_generation_permission_resolution or similar =="
rg -n -B5 -A60 'actor_generation_permission_resolution|actor_generation_terminalization_requires|provider_background_interaction_resolution' crates/orca-runtime/src/runtime_surface/commit.rs crates/orca-runtime/src/runtime_surface -g '*.rs' | sed -n '1,320p'Repository: echoVic/orca-agent
Length of output: 44954
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== InteractionPatch::Resolved definition =="
sed -n '620,670p' crates/orca-runtime/src/runtime_surface/projection.rs
sed -n '200,240p' crates/orca-runtime/src/runtime_surface/interaction.rs
echo
echo "== State lookup in provider_background_interaction_resolution_authorized =="
rg -n -B8 -A45 'fn provider_background_interaction_resolution_authorized' crates/orca-runtime/src/runtime_surface/commit.rs
echo
echo "== Tests for actor_generation_permission_resolution_authorized / close variants =="
rg -n -B15 -A80 'actor_generation_permission_resolution|actor_generation_terminalization_requires_matching_interaction_cancel_reason|actor_generation_interrupt_authority_is_exact_and_recoverable|provider_background_interaction_resolution_authorized' crates/orca-runtime/src/runtime_surface/commit.rs | sed -n '1,420p'Repository: echoVic/orca-agent
Length of output: 22339
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== InteractionPatch exact definition =="
rg -n -B8 -A18 'enum InteractionPatch|Resolved \{' crates/orca-runtime/src/runtime_surface/interaction.rs crates/orca-runtime/src/runtime_surface/projection.rs
echo
echo "== SurfaceInteractionKind definition =="
rg -n -B12 -A45 'enum SurfaceInteractionKind|SurfaceInteractionKind' crates/orca-runtime/src/runtime_surface/interaction.rs crates/orca-runtime/src/runtime_surface/projection.rs | sed -n '1,220p'
echo
echo "== Permission-related interaction kinds =="
rg -n -B5 -A35 'Permission|permission|ask_for|AskFor|Allow|allow|Deny|deny' crates/orca-runtime/src/runtime_surface/interaction.rs crates/orca-runtime/src/runtime_surface/projection.rs | sed -n '1,260p'
echo
echo "== test block names around authorized tests =="
rg -n -B3 -A3 '#\[test\]|fn .*required|fn .*authorized|fn .*correlated|actor_generation' crates/orca-runtime/src/runtime_surface/commit.rs | sed -n '2200,2550p'Repository: echoVic/orca-agent
Length of output: 50374
Require full correlation for permission-resolution batch authority.
actor_generation_permission_resolution_authorized only checks event discriminants and the generation fence. Since InteractionPatch::Resolved can also carry non-permission interactions and the authorizer is used for live commits and JSONL recovery, require that the resolved interaction is a PermissionRequest, that receipt.kind matches the interaction kind, and that the committed settings align with the resolved permission request/expected state. Mirror the interaction-state checks used by provider_background_interaction_resolution_authorized and add a covering test.
🤖 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/orca-runtime/src/runtime_surface/commit.rs` around lines 5190 - 5237,
Strengthen actor_generation_permission_resolution_authorized by extracting and
validating the committed settings and resolved interaction payload, requiring a
PermissionRequest, matching receipt.kind to the interaction kind, and confirming
the settings match the resolved request and expected state. Mirror the
interaction-state correlation checks in
provider_background_interaction_resolution_authorized, and add a covering test
for mismatched or non-permission resolutions, including JSONL recovery behavior.
… metadata authority Continue closing the metadata permission escape paths surfaced by the Seatbelt review. Session-scoped filesystem grants are now resolved and committed by the runtime in the same durable batch as the permission decision, so a failed append can neither mutate settings nor wake the waiting tool, and a later agent turn restores the exact dedicated metadata grant without reloading session history from the Bash execution path. Sandbox hardening: - Keep dedicated metadata write authority (`session-metadata` source) provenance-separate from ordinary writable roots so an ordinary grant can never be mistaken for metadata authority. - Linux/bubblewrap discovers existing nested protected metadata (`.git`, `.agents`, `.codex`) before launch and re-binds it read-only, refuses to let symlinked metadata roots widen an explicit grant, and fails closed when an overlapping policy requires bubblewrap but it is unavailable. - macOS/Seatbelt keeps workspace metadata read-only under writable roots by default. Also refresh the v0.3.0 release notes, changelog, and sitemap to document the new session-grant durability and metadata isolation behavior. Co-authored-by: TRAE CLI <noreply@bytedance.com>
…y manifest The Seatbelt hardening added `#[cfg(unix)]`/`#[cfg(all(test, unix))]` regression tests that use `std::os::unix::fs::symlink` to prove symlinked metadata roots cannot escalate into writable grants. The reviewed Windows platform boundary contract scans every `.rs` file for `std::os::unix::` usage and requires each occurrence to be registered with an exact count, so the unregistered fixtures failed the `Validate reviewed platform boundaries` gate on both native-x64 and native-arm64. Register the new unix-only test fixtures and correct one count the earlier metadata-escape commit left stale: - runtime_permission.rs: approved_symlink_metadata_root_is_not_made_writable - sandbox/bwrap.rs: symlinked_metadata_does_not_treat_its_writable_target... - sandbox/mod.rs: metadata_grant_rejects_symlinked_parent_components - sandbox/seatbelt.rs: bump unix_std_api 5 -> 6 for the added symlink test All entries are `#[cfg(unix)]`-gated fixtures (Windows symlink creation requires privilege), consistent with the existing runtime-surface-test-symlink boundary. Verified locally: `node scripts/validate-windows-platform-boundaries.mjs` and `node scripts/test-validate-windows-platform-boundaries.mjs` both pass. Co-authored-by: TRAE CLI <noreply@bytedance.com>
The async subagent contract tests capture the child `orca exec` streams, so when the launch exits non-zero on Windows the assertion collapses to `Some(1)` with no diagnostic. Add an `assert_launched_ok` helper that prints the captured stdout and stderr on mismatch to reveal the real failure reason from CI. Temporary diagnostic to root-cause the Windows async worker adoption path. Co-authored-by: TRAE CLI <noreply@bytedance.com>
52aed80 to
10bf009
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/orca-runtime/src/runtime_host.rs (1)
41726-41740: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTest asserts rejection but doesn't cover the bypass path.
Good coverage for the direct
ReplaceMetadataWritableDirectoriesrejection. Consider adding a sibling case that submitsReplaceAdditionalWorkingDirectorieswithsource: "session-metadata"for the same.gitpath, which is the escape route flagged at Lines 8068-8076.🤖 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/orca-runtime/src/runtime_host.rs` around lines 41726 - 41740, Extend the test around the existing unauthorized ReplaceMetadataWritableDirectories assertion to also submit ReplaceAdditionalWorkingDirectories for the same metadata path with source set to "session-metadata". Assert that this bypass attempt returns SurfaceClientCommandError::Unauthorized, while preserving the existing direct-replacement assertion.
🤖 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.
Nitpick comments:
In `@crates/orca-runtime/src/runtime_host.rs`:
- Around line 41726-41740: Extend the test around the existing unauthorized
ReplaceMetadataWritableDirectories assertion to also submit
ReplaceAdditionalWorkingDirectories for the same metadata path with source set
to "session-metadata". Assert that this bypass attempt returns
SurfaceClientCommandError::Unauthorized, while preserving the existing
direct-replacement assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bb50d8de-3ccc-4156-aee0-e6d093e77168
📒 Files selected for processing (32)
crates/orca-provider/src/lib.rscrates/orca-runtime/src/runtime_bash.rscrates/orca-runtime/src/runtime_host.rscrates/orca-runtime/src/runtime_permission.rscrates/orca-runtime/src/runtime_surface/commands.rscrates/orca-runtime/src/runtime_surface/commit.rscrates/orca-runtime/src/runtime_surface/operation.rscrates/orca-runtime/src/runtime_surface/reducer.rscrates/orca-runtime/src/server.rscrates/orca-runtime/src/server/command_exec_sandbox.rscrates/orca-runtime/src/server/processors/permission.rscrates/orca-runtime/src/server/surface_adapter.rscrates/orca-runtime/src/tool_router.rscrates/orca-runtime/tests/runtime_surface_attach.rscrates/orca-runtime/tests/runtime_surface_commit.rscrates/orca-runtime/tests/runtime_surface_interaction.rscrates/orca-runtime/tests/runtime_surface_reducer.rscrates/orca-tools/src/external.rscrates/orca-tools/src/sandbox/bwrap.rscrates/orca-tools/src/sandbox/linux.rscrates/orca-tools/src/sandbox/mod.rscrates/orca-tools/src/sandbox/seatbelt.rscrates/orca-tui/src/app.rsdocs/releases/v0.3.0.mddocs/superpowers/specs/2026-07-28-native-windows-platform-foundation.manifest.jsonsite/public/sitemap.xmlsite/src/changelog/Changelog.tsxsite/src/shared.tstests/server_runtime_contract.rstests/session_server_contract.rstests/subagent_contract.rstests/tui_pty_contract.rs
💤 Files with no reviewable changes (1)
- crates/orca-runtime/src/server/processors/permission.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/orca-runtime/tests/runtime_surface_attach.rs
- site/src/changelog/Changelog.tsx
- crates/orca-runtime/src/runtime_surface/commands.rs
1 participant
Summary
Closes the metadata permission escape paths surfaced by the Seatbelt security review. Contains the four review-hardening commits already on the branch plus a final change set that makes session-scoped filesystem grants durable and isolates dedicated metadata write authority from ordinary writable roots.
Changes
Runtime — atomic session grants
Sandbox hardening
session-metadatasource) is kept provenance-separate from ordinary writable roots, so an ordinary writable grant can never be mistaken for metadata authority..git,.agents,.codex) before launch and re-binds it read-only; refuses to let symlinked metadata roots widen an explicit grant; fails closed when an overlapping policy requires bubblewrap but it is unavailable.Also refreshes the v0.3.0 release notes, changelog, and sitemap to document the new behavior.
Verification
cargo build(workspace) — clean.cargo test -p orca-tools sandbox— 67 passed (includes metadata-escape, symlink, and descendant-grant hardening tests).cargo test -p orca-runtime --test runtime_surface_interaction -- --test-threads=1— 34 passed.cargo clippy --workspace --tests— no new errors.Note on parallel test flakiness
runtime_surface_interactionshows intermittent failures when run multi-threaded. This was confirmed to be pre-existing on the branch (reproduced on the branch tip without this change set) and is caused by shared-resource contention in the harness, not by this change. All tests pass single-threaded.Summary by CodeRabbit