Skip to content

feat: add kernel filesystem-policy engine adapter (feature-flagged) - #57

Open
kevin-orellana wants to merge 3 commits into
strands-agents:mainfrom
kevin-orellana:kernel-policy-bootstrap
Open

kevin-orellana wants to merge 3 commits into
strands-agents:mainfrom
kevin-orellana:kernel-policy-bootstrap

Conversation

@kevin-orellana

@kevin-orellana kevin-orellana commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

Kernel filesystem requests need the same Dogwood policy integration that already serves Strands Shell and the Egress component. This PR starts that integration behind an off-by-default feature flag.

Changes made

  1. The policy crate exports KernelPolicyAdapter from adapters/kernel.rs, behind the off-by-default kernel-adapter feature. Box's kernel-policy-integration feature enables it inside the trusted box process.
  2. The HostedBox component gives it the existing engine and box identity.
  3. Filesystem checks reuse Strands Shell's operation types, the shared agent principal (policy identity), and the existing context.input.path field. They retain the current path-reporting convention. We will evaluate Resource separately; any migration should update Strands Shell and kernel filesystem integration together.

Policy engine integration notes

The adapter preserves the existing policy contract, including path reporting and outcome recording. Recording errors reach the caller.

A separate PR will translate operating-system filesystem events into policy-engine actions and test compatibility with Strands Shell.

Release notes

This is bootstrap code. Even with the feature enabled, it installs no kernel hooks and provides no kernel filesystem protection. It adds no runtime activation switch, configuration key, policy vocabulary, sandbox grant, or network interception. Later PRs will connect the operating-system callbacks.

Please review the shared-engine ownership, existing policy contract, and feature-flagged build decision.

Related Issues

Part of strands-agents/staging-boxy#43 (private tracking repository; access required).

Type of Change

New feature: inactive integration foundation.

Testing

Unit tests:

Five new unit tests exercise the kernel-policy engine adapter:

  • Action, operation, principal, and path compatibility, including default denial.
  • Separate permission and outcome records for completed operations, issued file descriptors, indeterminate outcomes, and failed operations.
  • History propagation in both directions between the kernel-policy engine adapter and the existing Strands Shell adapter.
  • The kernel-policy engine adapter constructed by HostedBox submits to that box's existing engine.
  • A controlled recording error reaches the caller through the actual adapter method.

These are adapter tests. They do not yet demonstrate kernel filesystem interception.

Local validation used macOS 27.0.1:

  • After moving the adapter into the policy crate: 175 policy library tests passed, with seven existing tests ignored. The three kernel-only adapter tests and the HostedBox test passed. Policy and Box builds, formatting, lint, and public API documentation checks passed. The kernel-only production dependency tree excludes Shell, Monty, and the Egress component.
  • Workspace all-feature build, no-default-feature Box build, cargo fmt --all --check, and just clippy passed. The lint command uses the repository's allowances for vendored Shell warnings.
  • scripts/test-all.sh initially reported 4,040 passes, 92 failures, and 17 ignored tests. The review used an overlong temporary path, which exceeded macOS socket limits. All 92 affected cases passed when rerun with the standard temporary directory. Other environment-dependent skips remain; no scripted examples are present. This was not a green first invocation.
  • The separate deterministic suite passed 121 cases, with eight Linux-only cases skipped and full applicable coverage.
  • Deliberate defects in operation forwarding, outcome path spelling, shared history, fabricated completion, and recording-error propagation caused the new tests to fail.

Linux was not built or tested locally.

GitHub validation at this update:

Commit 504560e moves the adapter into the policy crate. GitHub validation for this revision is pending. See the PR checks for current results.

Kernel filesystem interception was not tested. The tests simulate a recording failure; actual storage-device failures were not injected.

  • I ran the relevant suites (just check, or cargo test --workspace --all-features)
  • I ran cargo fmt --all --check and just clippy, using the repository's documented lint allowances.

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • My changes generate no new warnings

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

## Problem

Native filesystem integration needs the existing Box policy authority. Landing that connection separately keeps platform enforcement out of this initial review.

## Solution

Add a private adapter behind the non-default kernel-policy-integration build feature. HostedBox passes its existing engine and box identity to that adapter.

Keep existing operation types, the fixed agent principal, and reported path spelling. Submit permission requests and outcome records separately. Return recording errors.

Neither default nor feature-enabled builds install kernel hooks. Configuration, sandbox grants, and policy vocabulary remain unchanged.

## Tests

The native_checks_keep_the_existing_actions_identity_and_path_spelling test pins action, operation, principal, and path compatibility.
The admission_does_not_record_completion_and_outcomes_keep_request_spelling test pins separate outcome records.
The shell_and_native_checks_share_one_history test pins history propagation through both real adapters.
The native_adapter_submits_to_the_hosted_authority test pins production construction with the shared engine.
The a_recording_error_reaches_the_caller test pins propagation of a controlled recording error.
Deliberate defects in these contracts caused the respective tests to fail.

On macOS 27.0.1, all-feature and no-default builds, format checks, and just clippy passed.
The workspace script reported 4,040 passes, 92 temporary-path failures, and 17 ignored tests. All 92 affected cases passed with a short temporary directory.
The deterministic suite passed 121 cases and skipped eight Linux-only cases. Environment-dependent workspace skips remain; no scripted examples exist.

Linux builds, native interception, and storage-error fault injection were not tested.
@kevin-orellana kevin-orellana changed the title feat: add inactive native filesystem policy adapter feat: add kernel filesystem-policy engine adapter (feature-flagged) Oct 9, 2026
Comment thread crates/box/src/run/native_policy.rs Outdated
#[cfg(not(test))]
let record = PolicyEngine::record;
#[cfg(test)]
let record = self.record_outcome;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: The #[cfg(not(test))] arm of record (let record = PolicyEngine::record;) is never compiled under cargo test, so the exact wiring that ships is never test-executed. The non-error tests exercise the functionally-equivalent default function pointer instead, and a_recording_error_reaches_the_caller overrides it. The two paths are equivalent today, but the seam means the shipped line and the tested line are not the same line.

Suggestion: Consider holding record_outcome in all builds (default PolicyEngine::record) rather than behind #[cfg(test)]. That removes the cfg split in record, and the production path becomes the one the tests drive. This is a minor, non-blocking point given the paths are equivalent and the PR already notes a real storage-device failure could not be injected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Looking into and considering adoption.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for turning this around in fix: share outcome recorder dispatch across builds. I re-reviewed the new revision (03c4936): record_outcome is now a field in all builds defaulting to PolicyEngine::record, and record dispatches through it with no cfg split — so the shipped path is the tested path, and the error-injection test still exercises the same seam. Re-verified locally: cargo fmt --check, clippy (0 box warnings, no dead-code on the always-present field), the feature build, and all 5 native tests (582 total, 0 failed) pass. This resolves the suggestion — nice.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Assessment: Approve

The PR delivers exactly what it claims: an inactive, feature-gated adapter that reuses the box's single policy authority and installs no kernel hooks. I built and tested it locally; findings below.

Review Categories
  • Interface freeze: Respected. box is a bin-only crate, the kernel-policy-integration feature is non-default and inactive, and nothing touches the CLI verbs, box.toml keys, record versions, or wire protocol. No public API surface is added, so no API review is required.
  • Shared-engine ownership: Confirmed. NativePolicy receives the same Arc<PolicyEngine> the run already owns (the final move of the policy Arc), so the "one Policy per box" premise and one temporal history hold. The cross-adapter history test with Shell demonstrates this.
  • Policy contract: Outcome recording matches the existing script.rs::record_fs_outcome pattern (reported path spelling, request-vs-outcome separation), keeping native checks consistent with the Shell/script adapters.
  • Verification: cargo fmt --check, cargo clippy (0 warnings owned by box), the default no-feature build (module correctly absent), the feature build, and all five new tests pass locally.
  • Minor: One non-blocking suggestion left inline on the record test seam.

Well-scoped foundation with tests that pin each behavioral claim, including deliberate-defect coverage.

@kevin-orellana kevin-orellana self-assigned this Oct 9, 2026
Comment thread crates/box/src/run/native_policy.rs Outdated
@github-actions

Copy link
Copy Markdown

Assessment: Approve

Re-reviewed after the restructure (504560e, "move kernel adapter into the policy crate"). The adapter moved from a pub(super) type in the box binary to a feature-gated public KernelPolicyAdapter in the policy crate, alongside the existing egress/shell/script adapters. The core logic is unchanged from the prior approved revision, and the record_outcome dispatch suggestion from my earlier review carried over cleanly — production and tests drive the same path. Re-verified all gates locally.

Review Categories
  • Placement & precedent: The move puts kernel policy integration next to its siblings (EgressPolicyInterceptor/ShellPolicyInterceptor/ScriptPolicyInterceptor), each public behind its own off-by-default feature. Gating is consistent: kernel-adapter guards both pub(crate) mod kernel and the pub use re-export, so the default policy build does not expose the type.
  • Inactivity preserved: box enables it via kernel-policy-integration = ["policy/kernel-adapter"], both off by default. HostedBox holds _kernel_policy built from the run's existing Arc<PolicyEngine> — one authority, one history — and still installs no kernel hooks.
  • Decision record: decisions.md documents the placement, the one-additional-public-type cost, and the user-approved interface change. Good traceability.
  • API surface: New public type — flagged a non-blocking process note inline re: the api/needs-review label vs. the documented approval.
  • Tests: Compatibility matrix, outcome spelling across all four FsResult variants, bidirectional shell↔kernel shared-history, HostedBox wiring, and deliberate recording-error propagation. Full equality/matches! assertions throughout.
  • Verification: cargo fmt --all --check; policy default build (type absent); policy clippy kernel-adapter,shell-adapter (0 warnings); 4 kernel tests + full policy suite; policy doc check; box default build; box clippy kernel-policy-integration (0 warnings); and kernel_adapter_submits_to_the_hosted_authority — all pass.

Clean relocation that improves cohesion with the other adapters while keeping the integration inactive and well-tested.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants