Skip to content

Commit b9f2454

Browse files
author
adpa-ms
committed
docs: address PR review -- stale validator comment, fragile test counts, policy.ui gap
state_aware.rs: the block comment above the validate_<phase> hooks still said network policy was rejected at every phase. That stopped being true when provision started accepting the canonical unrestricted-network acknowledgment. Correct the network clause while keeping the statements that are still true (the backend has no network primitive; proxy policy is rejected at every phase). oneshot.md: drop the Test Plan Count column and the '~31 backend-specific' / '287 total currently passing' figures. They were already stale (policy.rs has 28 tests, listed as ~24) and the rows never summed to the stated total. No other backend doc or README tracks test counts, so removing them makes this doc consistent and deletes a number that silently rots whenever a test is added. The durable Category / Location / What-it-verifies content stays. state-aware-rust.md: policy.ui was documented as rejected at every phase, but the backend never validates it, so a supplied UI policy is silently ignored. Correct the matrix to 'ignored' (matching the runtime and the state-aware design doc) and add an explicit known-gap note -- this is the same false-guarantee shape the network honesty gate closes, so rejecting policy.ui is the intended end state, not a deliberate exemption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb
1 parent cecce4c commit b9f2454

3 files changed

Lines changed: 32 additions & 21 deletions

File tree

docs/isolation-session/oneshot.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -239,16 +239,16 @@ versions and stating that the bindings must be regenerated.
239239

240240
**Automated (`cargo test`, runs on any machine including CI):**
241241

242-
| Category | Count | Location | What it verifies |
243-
|---|---:|---|---|
244-
| Config parsing | ~8 | `config_parser.rs` | `"isolation_session"` containment value and `experimental.isolation_session` section parsing |
245-
| Policy validation | ~24 | `policy.rs` | Filesystem fields (`readwritePaths` / `readonlyPaths` / `deniedPaths`) are rejected at every phase; the network policy must be the canonical unrestricted-network acknowledgment (`defaultPolicy=allow` + `allowLocalNetwork=true`, no host rules or proxy) at provision, and any supplied network policy is rejected post-provision |
246-
| Option building | ~6 | `isolation_session_runner.rs` | `ExecutionRequest``ProcessOptions` mapping (timeout, cwd, env vars, redirect flags) |
247-
| Feature unavailable | 1 | `isolation_session_runner.rs` | Runner returns a clean error on machines without the IsolationSession feature enabled, so the test passes everywhere |
248-
249-
These ~31 backend-specific tests run alongside the existing workspace tests
250-
(287 total currently passing). The feature-unavailable test is what runs in
251-
CI, since CI machines do not have a Windows build with the IsolationSession feature enabled.
242+
| Category | Location | What it verifies |
243+
|---|---|---|
244+
| Config parsing | `config_parser.rs` | `"isolation_session"` containment value and `experimental.isolation_session` section parsing |
245+
| Policy validation | `policy.rs` | Filesystem fields (`readwritePaths` / `readonlyPaths` / `deniedPaths`) are rejected at every phase; the network policy must be the canonical unrestricted-network acknowledgment (`defaultPolicy=allow` + `allowLocalNetwork=true`, no host rules or proxy) at provision, and any supplied network policy is rejected post-provision |
246+
| Option building | `isolation_session_runner.rs` | `ExecutionRequest``ProcessOptions` mapping (timeout, cwd, env vars, redirect flags) |
247+
| Feature unavailable | `isolation_session_runner.rs` | Runner returns a clean error on machines without the IsolationSession feature enabled, so the test passes everywhere |
248+
249+
These backend-specific tests run alongside the existing workspace tests. The
250+
feature-unavailable test is what runs in CI, since CI machines do not have a
251+
Windows build with the IsolationSession feature enabled.
252252

253253
**Integration tests (require a Windows host with the IsolationSession feature enabled):**
254254

docs/isolation-session/state-aware-rust.md

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ it accepts is the optional Entra `user` bundle, at provision and start.
132132
| `policy.filesystem.deniedPaths` | rejected | rejected | rejected | rejected | rejected |
133133
| `policy.network` — canonical `allow` acknowledgment (`defaultPolicy=allow` + `allowLocalNetwork=true`, no host rules, no proxy, default enforcement) | **required** | rejected | rejected | rejected | rejected |
134134
| `policy.network` — any other value (incl. absent → `block`, host rules, proxy) | rejected | rejected | rejected | rejected | rejected |
135-
| `policy.ui` | rejected | rejected | rejected | rejected | rejected |
135+
| `policy.ui` — see the known gap below | ignored | ignored | ignored | ignored | ignored |
136136
| `experimental.isolation_session.{provision,start}.user` | **honored** | **honored** | n/a | n/a | n/a |
137137

138138
Rejection of `policy.*` fields surfaces as `error.code = "policy_validation"`.
@@ -155,13 +155,20 @@ user it assigned at provision.
155155
### Policy fields and mode parity
156156

157157
Both modes share the same policy matrix above. Every `policy.filesystem`
158-
field (`readwritePaths`, `readonlyPaths`, `deniedPaths`) and `policy.ui` are
159-
rejected at every phase (no host-folder-sharing primitive; `ui` not yet
160-
honored). The network policy is honesty-gated per the matrix — provision
161-
requires the canonical unrestricted-network acknowledgment and post-provision
162-
rejects any supplied network policy (inheriting an absent one). One-shot
163-
enforces this via `validate_runner`; state-aware enforces it via the
164-
`validate_<phase>` hooks.
158+
field (`readwritePaths`, `readonlyPaths`, `deniedPaths`) is rejected at every
159+
phase (no host-folder-sharing primitive). The network policy is honesty-gated
160+
per the matrix — provision requires the canonical unrestricted-network
161+
acknowledgment and post-provision rejects any supplied network policy
162+
(inheriting an absent one). One-shot enforces this via `validate_runner`;
163+
state-aware enforces it via the `validate_<phase>` hooks.
164+
165+
**Known gap — `policy.ui` is silently ignored.** The backend has no
166+
UI-restriction primitive, and it does not validate `policy.ui`, so a supplied
167+
UI policy is accepted and then dropped rather than refused. That is the same
168+
false-guarantee shape the network honesty gate closes: a caller asking for a
169+
UI restriction gets no error and no enforcement. Rejecting `policy.ui` is the
170+
intended end state, not a deliberate exemption — the matrix records `ignored`
171+
because that is the current behavior, not the desired one.
165172

166173
### Fields valid in state-aware only
167174

src/backends/isolation_session/common/src/state_aware.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,13 @@ impl StatefulSandboxBackend for IsolationSessionRunner {
156156
Ok(DeprovisionResult { metadata: None })
157157
}
158158

159-
// Filesystem rw/ro/denied paths, network, and proxy policy are rejected
160-
// at every phase: the backend has no host-folder-sharing, network, or
161-
// proxy primitive. Anything rejected produces a `policy_validation`
159+
// Filesystem rw/ro/denied paths are rejected at every phase: the backend
160+
// has no host-folder-sharing primitive. Network policy is honesty-gated —
161+
// the backend cannot filter or deny the container network, so provision
162+
// requires the canonical unrestricted-network acknowledgment, and every
163+
// post-provision phase rejects a supplied network policy (the posture is
164+
// fixed at provision) while inheriting an absent one. Proxy policy is
165+
// rejected at every phase. Anything rejected produces a `policy_validation`
162166
// envelope rather than silent ignore.
163167

164168
fn validate_provision(

0 commit comments

Comments
 (0)