diff --git a/CHANGELOG.md b/CHANGELOG.md index 44bb90d..6ecbfe8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to ocserve. The wire contract targets the frozen opencode 1.19.5 release and the v2 surface the 2.2.5 client; anything that differs is a named `D-*` row in the ledger at `PLAN.md` section 18. +## Unreleased + +### Fixed +- Permission requests now serialize in the correct v1 or v2 shape at each API + boundary, even when both API families share the same pending request. V2 + permission creation also rejects fields and values outside the frozen + request schema before storing them. + ## 0.7.1 - 2026-11-10 ### Added diff --git a/PLAN.md b/PLAN.md index f0fca14..e0dc474 100644 --- a/PLAN.md +++ b/PLAN.md @@ -615,6 +615,8 @@ current anywhere. | `D-WIN-WAL-NETWORK` | A data directory on a network filesystem is refused at boot rather than degraded. | accepted | | `D-MACOS-TRIM` | `trim_heap()` is a no-op off glibc unless measurement justifies otherwise. | open | | `D-TOOLS-TASK` | The `task` subagent tool was advertised by the frozen provider but not implemented. | closed (the tool runs on the shared executor with a depth guard) | +| `D-PERM-SOURCE-KEY` | The permission `source` object uses different keys per contract line: the v1 (1.19.5) SDK type `PermissionV2Source` is `{type,messageID,callID}` while v2 (2.2.5) `Permission.Source` is `{type,messageID,id}`. ocserve emits `id` on the v2 wire and `callID` on the v1 wire; the gate stores the call id under `callID` and each projector renames it. A body cannot satisfy both schemas because both are `additionalProperties:false`. | open | +| `D-PERM-EVENT-NAME` | Upstream 1.19.5 names the v2 permission event `permission.v2.asked`; 2.2.5 names it `permission.asked`. ocserve keeps the v1 name `permission.asked` on every stream and projects the v2 *payload* on `/api/event`, so a 1.19.5-era consumer that dispatches on `permission.v2.asked` will not see it. No shipped 1.19.5 client dispatches on that event: the TUI refreshes permissions over REST, and the web app's adapter maps `permission.v2.asked` only when the server sends it. The 1.19.5/2.2.5 web app also probes `/global/health` first and treats any `{healthy:true}` answer as a v1 server (`server-protocol.ts`), so against ocserve it subscribes to `/global/event` and consumes the v1 payload — the v2 permission frame is reachable only from a client that calls `/api/event` directly. | open | Historical rows kept for the tokens' own history, all closed: `D-GENERATE-EPHEMERAL`, `D-REVERT-NOSNAP`, `D-VCS-NOAPPLY`, `D-HOOK-*` items are in §16.9 and diff --git a/TESTING.md b/TESTING.md index 44f1793..10f8eff 100644 --- a/TESTING.md +++ b/TESTING.md @@ -231,6 +231,7 @@ event types), the two antagonism ledgers (every finding = at least one test). | a built-in command is advertised but not invocable (2026-11-05, reported by a real client: `/goal ` failed with "Command failed"). The built-in entry is advertised with `template: ""` because the v1 `Command` schema requires the field, and the dispatcher tested `template.is_none()` — true only in the test fixture's EMPTY command list, never in production, so the call fell through to `expand_command("")`, sent the model an EMPTY prompt and 500'd; the v2 path had a sibling bug (`command disappeared`) | the predicate is "missing OR empty" and the v2 dispatch matches the built-in by name before its template lookup; guard rule 69 fails on `template.is_none()` as a dispatch test | `tests/goal_e2e.rs` fixtures are built with the PRODUCTION payload shape (`template: ""` advertised) and the regression test asserts the response is `SessionV1.WithParts`; planted negative control: restore `is_none()` → 5 red; live proof on the release binary + stub provider: HTTP 200 in 24 ms, `info.role=assistant`, 3 parts, goal armed | | model-driven reviewer text treated as instructions (2026-11-05 security review H1: the continuation template interpolated `${feedback}` RAW — only the objective was escaped — so a hostile repository could steer the judge into emitting `{"verdict":"continue","reason":""}`, which was persisted as a user-role message and replayed with tools enabled, surviving compaction) | `continuation()` escapes and wraps the verdict in ``; reviewer prose is capped (`MAX_FEEDBACK`); `judge_prompt` escapes shell commands too | `reviewer_feedback_is_escaped_and_wrapped`, `reviewer_feedback_is_bounded`, `judge_escapes_shell_commands` (a `` in the reason must not close the block) | | an autonomous loop that keeps spending after the user stopped it (2026-11-05 reviews M1/M2/M3: `/abort`, v2 `interrupt` and v2 session delete aborted only `prompt_tasks`; the goal driver is a bare task, so the turn finished, the reviewer said continue, and another turn ran; `/abort` also left the row ACTIVE so the next idle hook restarted the loop) | v1 abort, v2 interrupt and v2 delete all `cancel_driver` + pause the goal with "Stopped by the user" (upstream pauses on a user interrupt); drivers re-check status AND objective before every turn; a `Drop` guard removes the registry entry on every exit path; a failed context/lock start PAUSES instead of leaving ACTIVE-with-no-driver | `abort_pauses_the_goal_and_stops_the_loop` (deterministic: the provider holds the kickoff open on a gate so the abort lands mid-turn; asserts status paused, rounds frozen, zero further reviewer calls) with a proven negative control (remove the pause → red) | +| bundled v2 permission UI crashed on `request.resources.length` because pending asks created by v1 tool calls and v2 permission routes crossed the shared gate without boundary projection; malformed v2 fields could also create requests outside the frozen schema | v1 and v2 permission projectors map gate values into each frozen request shape, filter string arrays, and only project a tool source; v2 create validates required string arrays plus optional metadata/source and rejects unknown keys (including `message`) before persistence; the shared always-grant/cascade logic accepts `always/patterns` and `save/resources`; `/api/event` and v2 reads use v2, while `/permission` uses v1 | `v2_surface.rs::v2_permission_requests_project_v1_shape_for_reads`, `permission_create_evaluates_ruleset` (checks both read shapes, then replies always and verifies grant/cascade), `permission_create_rejects_malformed_request_fields_before_persisting`, `event_envelope_unit::v2_permission_event_projects_the_permission_request_shape`, `event::tests::permission_request_projects_v2_gate_values_into_the_v1_schema`, `event::tests::v2_permission_projection_drops_malformed_fields_and_source_types`; negative controls bypass projectors, pass malformed entries/source types, accept an unknown create key, or read only v1 gate fields | | goal state writes swallowed (2026-11-05 SRE review: `set_goal` results discarded with `let _ =` in the boot scan, the round increment and `settle`; a persistently failing round write silently disables the 50-round cap, and a failed pause leaves ACTIVE-with-no-driver) | every goal write propagates or logs+counts (`ocserve_goal_total{event="persist_failed"}`); boot-scan failure skips the row loudly | `goal_state` suite; the store's single-writer discipline unchanged (all writes go through `Writer`) | | an autonomous loop the operator cannot see or stop (2026-11-05 SRE review: no reviewer budget, no cap on concurrent drivers, reviewer spend invisible, failures not surfaced to clients) | `OCSERVE_GOAL_REVIEW_SECS` (default 300 s) bounds each reviewer call so a chatty provider cannot hold the wake gate; `MAX_CONCURRENT_DRIVERS` (=4) bounds loops process-wide and PAUSES the ones that cannot start; reviewer tokens land in `ocserve_transient_tokens_total{owner="session"}` via `RunOpts::attribution_session`; a failed goal turn emits the same durable `session.error` shape `run_prompt` does | `review_budget` + env contract row (regenerated `docs/ENVIRONMENT.md`); the abort e2e asserts frozen rounds; `ocserve doctor` prints the switch and active goals | ## 3. Test levels (ISTQB) mapped to ocserve diff --git a/TRACEABILITY.md b/TRACEABILITY.md index b5f79ee..938db35 100644 --- a/TRACEABILITY.md +++ b/TRACEABILITY.md @@ -262,6 +262,8 @@ no-inflation correct), PASS cost (−58.9% weighted) + score (median 1.0 both) | K-FREEZE-1195 | the v1 freeze must be adopted through the pipeline, not by hand, and every artifact that embeds the tag must move with it | Moved 1.18.31 -> 1.19.5 with `bump.sh v1`. Contract: 188 operations over the same 162 paths (byte-diff: 2 schema descriptions, 1 additive `reasoning_options` property, `x-codeSamples` added to every operation). Prompt/tool text for everything the server sends is byte-identical (`tool/shell/shell.txt` unchanged, `bashCommandSection` unchanged); `plan-enter.txt` was DELETED upstream and is removed here. ARITY rules identical. Two live capture timings were also fixed: `acquire-freeze` printed success while `cp` failed on the root-owned vendored path (step 2 then booted the OLD binary), and `extract-prompts` had no remote fallback. | `capdiff` GREEN with the provider-header oracle enabled: it found the two real deltas (composite UA runtime segment `bun/1.3.14 -> 1.4.4`; `x-opencode-session-id` now sent to EVERY provider, not just `opencode*`), both fixed; replay 26/0; pair 18 compared / 0 divergent, 52 schema checks / 0 violations; `render_check` (prompt parity) green; `global_health` golden now `1.19.5`; test `composite_ua_matches_the_captured_artifact` pins the UA to a captured file, so a hand-written literal cannot drift again | green | | K-MUTANTS | mutation baseline + gate (Phase 2) | `snapshot::tests::non_git_directory_is_inactive` made hermetic (fixture plants a bogus `.git` boundary — the test falsely passed only when TMPDIR sat outside any repo; the mutants harness sets TMPDIR inside the repo, aborting the baseline). **Full-scope baseline (i9, 2330 mutants, 34 min): 939 caught / 1221 missed / 24 timeout / 146 unviable = 43.5 % score** — `bench/mutants/BASELINE.md` + `baseline-outcomes.json`; zero env-gated skips verified (1 intentional ignore). `scripts/check-mutants.py` gates missed count against ceiling 1221 (ratchet-down-only; selftest red/green proven); `scripts/merge-mutants.py` combines `--shard` files with a duplicate guard (selftest red/green proven). Method corrected on evidence: `additional_cargo_test_args=["--lib"]` removed (drops store integration binaries → 933 false survivors, and lands `--lib` on the test binary). Acceleration: `.cargo/mutants.toml` profile=mutants/debug=none + 45s timeout floor; runs on the i9 (`-j8`, mold, sccache, tmpfs) | green | +| K-V2-PERM-SHAPE | pending permission requests use each frozen grammar on reads and events, regardless of which API family created them, while shared always-grant behavior accepts both grammars | The permission gate is shared: v1 tool calls store `permission`/`patterns`/`always`/`tool`, while v2 creates store `action`/`resources`/`save`/`source`. V2 create validates string-array members plus metadata/source shapes, and rejects unknown fields such as `message` because the frozen request schema has `additionalProperties: false`. Bidirectional projectors whitelist fields, filter string arrays, and emit `source` only for the frozen `tool` variant. The gate's always-reply and cascade logic reads either vocabulary so v2 asks grant and resolve matching pending requests without leaking fields across wire shapes. | `v2_surface.rs::v2_permission_requests_project_v1_shape_for_reads` covers global/session/detail and isolation; `v2_surface.rs::v2_created_ask_is_readable_in_both_grammars` drives the REAL POST /api/session/{id}/permission create path and asserts the same stored ask reads back with `source.id` on the v2 route and `tool.callID` on the v1 route; `permission_create_evaluates_ruleset` creates v2 asks, checks both list shapes, replies `always`, and verifies the session grant plus matching-request cascade; `permission_create_rejects_malformed_request_fields_before_persisting` covers malformed resources/save entries, optional metadata/source fields, and rejects the non-schema `message` field; `event_envelope_unit::v2_permission_event_projects_the_permission_request_shape` checks v2 event projection and unchanged v1 event; `event::tests::permission_request_projects_v2_gate_values_into_the_v1_schema` pins reverse projection; `event::tests::v2_permission_projection_drops_malformed_fields_and_source_types` covers malformed shared values. Negative controls: reading only v1 `always`/`patterns` makes the v2 grant and cascade assertions red; bypass either projector, allow bad nested payload fields, pass a non-tool source type, or accept an unknown permission-create key breaks the shape assertions. | green | + | K-CI-PIN | CI must fail for repository changes, not for a Rust release rolling onto the stable channel | On 2026-11-13 every workflow on `main` and on all fork PRs failed within two minutes with zero tests run. rustc 1.100.0 had landed on the runner and introduced cargo manifest lints (`unused_dependencies`, `unused_workspace_dependencies`, `redundant_homepage`); `CARGO_BUILD_WARNINGS=deny` turned them fatal. Reproduced locally under 1.100.0 with the same env (exit 101) and green under 1.99.0, proving the trigger was the compiler, not the code. Fixed by pinning `rust-toolchain.toml` to 1.100.0 (a floating channel makes `main` hostage to a release) and removing the declarations the lint proved unused (workspace `hyper`, `metrics`, `metrics-exporter-prometheus`, `thiserror`; per-crate `thiserror`, `rustc-hash`, `metrics`) plus the duplicated `homepage`. Also replaced the `current_exe()` binary walk in three process tests with `CARGO_BIN_EXE_ocserve`, which is what CI and remote builds actually need; the walk only worked when the test exe sat beside the bin. | `rust-toolchain.toml` pin; `cargo +1.100.0 clippy --workspace --all-targets -- -D warnings` under `CARGO_BUILD_WARNINGS=deny` clean; 907 tests pass on the build box; guards 110/110, matrix, docs green | green | ## Test design techniques coverage (TESTING §4) diff --git a/crates/ocserve-core/src/event.rs b/crates/ocserve-core/src/event.rs index 9fd8957..e03e306 100644 --- a/crates/ocserve-core/src/event.rs +++ b/crates/ocserve-core/src/event.rs @@ -346,11 +346,20 @@ pub fn normalize_props(event_type: &str, mut props: Value) -> Value { /// - `message.updated` (assistant) → `session.step.started` /// - `session.status` busy/idle → `session.status` (v2 shape) /// +/// `permission.asked` is NOT here: it is durable-only and is projected at the +/// `/api/event` encoder instead (see the note in the body). +/// /// Returns `None` for events with no v2 twin. Deltas are ephemeral /// (live-only, never persisted — matches upstream's ephemeral Delta defs); /// lifecycle boundaries are durable (the choke persists them). /// Unmapped events stay v1-only (the client ignores them, no lie). pub fn v2_twin(event_type: &str, properties: &Value) -> Option<(String, Value, bool)> { + // NOTE: no `permission.asked` arm here on purpose. That event is emitted + // only through `emit_durable`/`emit_durable_http`, and neither calls + // v2_twin; a twin added here would be unreachable code whose test could + // never exercise production. The v2 permission *payload* is projected at + // the `/api/event` encoder (`encode_event_frame` -> permission_request_v2), + // which is the path the envelope unit test covers. let part = properties.get("part")?; match event_type { "message.part.updated" => { @@ -408,6 +417,173 @@ pub fn v2_twin(event_type: &str, properties: &Value) -> Option<(String, Value, b } } +/// Project ocserve's v1 permission-gate value into the frozen v2 +/// `Permission.Request` shape. The gate is shared by both API families, so +/// keep its stored request untouched and apply this only at v2 boundaries. +pub fn permission_request_v2(request: &Value) -> Value { + let mut projected = serde_json::Map::new(); + projected.insert( + "id".into(), + request + .get("id") + .and_then(Value::as_str) + .map(Value::from) + .unwrap_or_else(|| json!("")), + ); + projected.insert( + "sessionID".into(), + request + .get("sessionID") + .and_then(Value::as_str) + .map(Value::from) + .unwrap_or_else(|| json!("")), + ); + projected.insert( + "action".into(), + request + .get("action") + .filter(|value| value.is_string()) + .or_else(|| request.get("permission")) + .and_then(Value::as_str) + .map(Value::from) + .unwrap_or_else(|| json!("")), + ); + projected.insert( + "resources".into(), + request + .get("resources") + .or_else(|| request.get("patterns")) + .and_then(project_permission_strings) + .unwrap_or_else(|| json!([])), + ); + + if let Some(save) = request + .get("save") + .or_else(|| request.get("always")) + .and_then(project_permission_strings) + { + projected.insert("save".into(), save); + } + if let Some(metadata) = request.get("metadata").filter(|value| value.is_object()) { + projected.insert("metadata".into(), metadata.clone()); + } + if let Some(message) = request.get("message").filter(|value| value.is_string()) { + projected.insert("message".into(), message.clone()); + } + if let Some(source) = request + .get("source") + .filter(|value| value.get("type").and_then(Value::as_str) == Some("tool")) + { + let mut source_out = serde_json::Map::new(); + source_out.insert("type".into(), json!("tool")); + for key in ["messageID", "id"] { + if let Some(value) = source.get(key).filter(|value| value.is_string()) { + source_out.insert(key.into(), value.clone()); + } + } + if source_out.len() == 3 { + projected.insert("source".into(), Value::Object(source_out)); + } + } else if let Some(tool) = request.get("tool").filter(|value| value.is_object()) { + let message_id = tool.get("messageID").and_then(Value::as_str); + let call_id = tool.get("callID").and_then(Value::as_str); + if let (Some(message_id), Some(call_id)) = (message_id, call_id) { + projected.insert( + "source".into(), + json!({"type": "tool", "messageID": message_id, "id": call_id}), + ); + } + } + Value::Object(projected) +} + +/// Project a shared gate value into the frozen v1 `PermissionRequest` shape. +/// V2 creates can share the gate with v1 callers, so v1 reads must translate +/// action/resources/save back and strip v2-only fields before serialization. +pub fn permission_request_v1(request: &Value) -> Value { + let mut projected = serde_json::Map::new(); + for key in ["id", "sessionID"] { + projected.insert( + key.into(), + request + .get(key) + .filter(|value| value.is_string()) + .cloned() + .unwrap_or_else(|| json!("")), + ); + } + projected.insert( + "permission".into(), + request + .get("permission") + .filter(|value| value.is_string()) + .or_else(|| request.get("action")) + .filter(|value| value.is_string()) + .cloned() + .unwrap_or_else(|| json!("")), + ); + for (output, sources) in [ + ("patterns", ["patterns", "resources"]), + ("always", ["always", "save"]), + ] { + let value = sources + .iter() + .find_map(|key| request.get(*key).and_then(project_permission_strings)) + .unwrap_or_else(|| json!([])); + projected.insert(output.into(), value); + } + projected.insert( + "metadata".into(), + request + .get("metadata") + .filter(|value| value.is_object()) + .cloned() + .unwrap_or_else(|| json!({})), + ); + + let tool = request + .get("tool") + .filter(|value| value.is_object()) + .cloned() + .or_else(|| { + let source = request.get("source").filter(|value| value.is_object())?; + if source.get("type")?.as_str()? != "tool" { + return None; + } + let message_id = source.get("messageID").and_then(Value::as_str)?; + let call_id = source.get("id").and_then(Value::as_str)?; + Some(json!({ + "messageID": message_id, + "callID": call_id, + })) + }); + if let Some(tool) = tool { + let mut tool_out = serde_json::Map::new(); + for key in ["messageID", "callID"] { + if let Some(value) = tool.get(key).filter(|value| value.is_string()) { + tool_out.insert(key.into(), value.clone()); + } + } + if tool_out.len() == 2 { + projected.insert("tool".into(), Value::Object(tool_out)); + } + } + Value::Object(projected) +} + +/// Keep permission string-array fields wire-valid even if a legacy or +/// malformed gate value reaches a projector. +fn project_permission_strings(value: &Value) -> Option { + let values = value.as_array()?; + Some(Value::Array( + values + .iter() + .filter_map(Value::as_str) + .map(Value::from) + .collect(), + )) +} + /// Encode a v2-grammar live frame (`{id,type,data}` — the `/api/event` /// envelope; `data` is the frozen V2 field name). pub fn frame_v2(event_type: &str, data: Value) -> Value { @@ -494,10 +670,67 @@ mod tests { assert_eq!(data["name"], "read"); // unmapped: no twin, never fabricated. assert!(v2_twin("session.updated", &json!({})).is_none()); + // permission.asked is durable-only (emit_durable/emit_durable_http never + // call v2_twin); its v2 shape is projected by the /api/event encoder. + // Asserting None here keeps a dead arm from being re-added: if someone + // maps it here again, this reds and the encoder test is the real proof. + assert!(v2_twin("permission.asked", &json!({"id": "per_1"})).is_none()); assert!(v2_twin("message.part.updated", &json!({"part": {"type": "patch"}})).is_none()); // CONTROL: map patch parts to a twin → the last assertion reds. } + #[test] + fn permission_request_projects_v2_gate_values_into_the_v1_schema() { + let request = json!({ + "id": "per_v2", + "sessionID": "ses_v2", + "action": "webfetch", + "resources": ["https://example.test"], + "save": ["https://example.test"], + "metadata": {"url": "https://example.test"}, + "source": {"type": "tool", "messageID": "msg_1", "id": "call_1"}, + "agent": "private-agent", + }); + let projected = permission_request_v1(&request); + assert_eq!( + projected, + json!({ + "id": "per_v2", + "sessionID": "ses_v2", + "permission": "webfetch", + "patterns": ["https://example.test"], + "always": ["https://example.test"], + "metadata": {"url": "https://example.test"}, + "tool": {"messageID": "msg_1", "callID": "call_1"}, + }) + ); + // Negative control: passing the stored value through leaks `action`, + // `resources`, and `source` and omits required v1 `permission`. + } + + #[test] + fn v2_permission_projection_drops_malformed_fields_and_source_types() { + let request = json!({ + "id": "per_v2", + "sessionID": "ses_v2", + "action": "webfetch", + "resources": ["https://example.test", 7], + "save": [false], + "metadata": "not-an-object", + "source": {"type": "user", "messageID": "msg_1", "id": "call_1"}, + "message": 3, + }); + + let projected = permission_request_v2(&request); + assert_eq!(projected["resources"], json!(["https://example.test"])); + assert_eq!(projected["save"], json!([])); + assert!(projected.get("metadata").is_none()); + assert!(projected.get("source").is_none()); + assert!(projected.get("message").is_none()); + // Negative control: passing array elements through and copying any + // string source.type makes these assertions violate Permission.Request. + } + /// normalize_props injects the required `time` on message.part.updated /// (spec Event.message.part.updated requires {sessionID, part, time}); /// ocserve omitted it → the 2026-10-09 TUI crash class. Idempotent. diff --git a/crates/ocserve-core/src/permission.rs b/crates/ocserve-core/src/permission.rs index 36278e9..79b3ae7 100644 --- a/crates/ocserve-core/src/permission.rs +++ b/crates/ocserve-core/src/permission.rs @@ -208,7 +208,11 @@ impl PermissionGate { .or_else(|| req.get("action")) .and_then(|v| v.as_str()) .unwrap_or(""); - if let Some(always) = req.get("always").and_then(|v| v.as_array()) { + if let Some(always) = req + .get("always") + .or_else(|| req.get("save")) + .and_then(|v| v.as_array()) + { for pat in always.iter().filter_map(|p| p.as_str()) { self.grant_always(sid, permission, pat); } @@ -307,7 +311,11 @@ fn patterns_covered(request: &Value, granted: &[GrantRule]) -> bool { .or_else(|| request.get("action")) .and_then(|v| v.as_str()) .unwrap_or(""); - let Some(patterns) = request.get("patterns").and_then(|v| v.as_array()) else { + let Some(patterns) = request + .get("patterns") + .or_else(|| request.get("resources")) + .and_then(|v| v.as_array()) + else { return false; }; !patterns.is_empty() diff --git a/crates/ocserve-http/src/lib.rs b/crates/ocserve-http/src/lib.rs index f49c8ce..f3a2c78 100644 --- a/crates/ocserve-http/src/lib.rs +++ b/crates/ocserve-http/src/lib.rs @@ -1424,7 +1424,13 @@ pub(crate) fn validate_permission_reply_key( /// GET /permission — pending permission requests. async fn get_permissions(State(st): State>) -> Json { - Json(serde_json::Value::Array(st.gate.list())) + Json(Value::Array( + st.gate + .list() + .iter() + .map(ocserve_core::event::permission_request_v1) + .collect(), + )) } /// GET /vcs — {branch, default_branch} (values cwd-dependent; keys golden). @@ -5570,9 +5576,14 @@ fn encode_event_frame(stored: &str, fmt: EventFormat) -> Option { let id = payload.get("id").cloned().unwrap_or(Value::Null); let ty = payload.get("type").cloned().unwrap_or(Value::Null); let props = payload.get("properties").cloned().unwrap_or(json!({})); + let data = if ty.as_str() == Some("permission.asked") { + ocserve_core::event::permission_request_v2(&props) + } else { + props.clone() + }; Some(match fmt { EventFormat::Bare => json!({"id": id, "type": ty, "properties": props}).to_string(), - EventFormat::V2 => json!({"id": id, "type": ty, "data": props}).to_string(), + EventFormat::V2 => json!({"id": id, "type": ty, "data": data}).to_string(), EventFormat::Global => unreachable!(), }) } @@ -7075,6 +7086,30 @@ mod event_envelope_unit { "v2 folds a v1 frame into data (documented projection)" ); } + + #[test] + fn v2_permission_event_projects_the_permission_request_shape() { + let v1 = r#"{"directory":"/w","project":"global","payload":{"id":"evt_perm","type":"permission.asked","properties":{"id":"per_1","sessionID":"ses_1","permission":"bash","patterns":["echo *"],"always":["echo *"],"metadata":{"command":"echo hi"},"tool":{"messageID":"msg_1","callID":"call_1"}}}}"#; + let encoded = encode_event_frame(v1, EventFormat::V2).expect("v2 permission event"); + let frame: serde_json::Value = serde_json::from_str(&encoded).unwrap(); + assert_eq!(frame["type"], "permission.asked"); + assert_eq!(frame["data"]["action"], "bash"); + assert_eq!(frame["data"]["resources"], serde_json::json!(["echo *"])); + assert_eq!(frame["data"]["save"], serde_json::json!(["echo *"])); + assert_eq!(frame["data"]["source"]["type"], "tool"); + assert_eq!(frame["data"]["source"]["messageID"], "msg_1"); + assert_eq!(frame["data"]["source"]["id"], "call_1"); + assert!(frame["data"].get("patterns").is_none()); + + let bare = encode_event_frame(v1, EventFormat::Bare).expect("v1 permission event"); + let frame: serde_json::Value = serde_json::from_str(&bare).unwrap(); + assert_eq!(frame["properties"]["permission"], "bash"); + assert_eq!( + frame["properties"]["patterns"], + serde_json::json!(["echo *"]) + ); + assert!(frame["properties"].get("resources").is_none()); + } } #[cfg(test)] diff --git a/crates/ocserve-http/src/v2.rs b/crates/ocserve-http/src/v2.rs index 2097d66..147545f 100644 --- a/crates/ocserve-http/src/v2.rs +++ b/crates/ocserve-http/src/v2.rs @@ -3736,10 +3736,22 @@ pub async fn permission_saved( /// GET /api/permission/request — `{location, data:[...]}` pending asks. pub async fn permission_request(State(st): State>) -> impl IntoResponse { - let data = st.gate.list(); + let data = project_permission_requests(&st, None); Json(json!({"location": api_location(&st), "data": data})) } +fn project_permission_requests(st: &AppState, session_id: Option<&str>) -> Vec { + st.gate + .list() + .into_iter() + .filter(|request| { + session_id + .is_none_or(|sid| request.get("sessionID").and_then(Value::as_str) == Some(sid)) + }) + .map(|request| ocserve_core::event::permission_request_v2(&request)) + .collect() +} + /// GET /api/question/request — `{location, data:[...]}` pending questions. pub async fn question_request(State(st): State>) -> impl IntoResponse { let data = st.question_gate.list(); @@ -3749,9 +3761,9 @@ pub async fn question_request(State(st): State>) -> impl IntoRespo /// GET /api/session/{sessionID}/permission — `{data:[]}` (per-session asks). pub async fn session_permission( State(st): State>, - Path(_id): Path, + Path(id): Path, ) -> impl IntoResponse { - Json(json!({"data": st.gate.list()})) + Json(json!({"data": project_permission_requests(&st, Some(&id))})) } /// GET /api/session/{sessionID}/question — `{data:[]}` (per-session questions). @@ -5221,7 +5233,7 @@ pub async fn question_reject( /// GET /api/session/{sessionID}/permission/{requestID} — one ask. pub async fn session_permission_one( State(st): State>, - Path((_sid, rid)): Path<(String, String)>, + Path((sid, rid)): Path<(String, String)>, ) -> Result, ApiError> { if !rid.starts_with("per") { return Err(ApiError { @@ -5230,13 +5242,14 @@ pub async fn session_permission_one( message: format!("Expected a string starting with \"per\", got \"{rid}\""), }); } - let found = st - .gate - .list() - .into_iter() - .find(|p| p.get("id").and_then(|v| v.as_str()) == Some(rid.as_str())); + let found = st.gate.list().into_iter().find(|p| { + p.get("id").and_then(Value::as_str) == Some(rid.as_str()) + && p.get("sessionID").and_then(Value::as_str) == Some(sid.as_str()) + }); match found { - Some(p) => Ok(Json(json!({"data": p}))), + Some(p) => Ok(Json(json!({ + "data": ocserve_core::event::permission_request_v2(&p) + }))), None => Err(ApiError::not_found(format!( "Permission request not found: {rid}" ))), @@ -5539,11 +5552,55 @@ pub async fn session_event(State(st): State>) -> axum::response::R /// ocserve's gate is a rendezvous for the RUNNING prompt; a standalone /// create is acknowledged with the evaluated effect (freeze shape /// `{data:{id,effect}}`). Full blocking semantics belong to the prompt path. +fn invalid_permission_field(path: &str, expected: &str, value: &Value) -> HttpError { + HttpError::TaggedData { + status: StatusCode::BAD_REQUEST, + tag: "InvalidRequestError", + fields: json!({ + "message": format!("Expected {expected}, got {value}\n at {path}"), + "kind": "Payload", + }), + } +} + +fn validate_permission_string_array(key: &str, values: &[Value]) -> Result<(), HttpError> { + for (index, value) in values.iter().enumerate() { + if !value.is_string() { + return Err(invalid_permission_field( + &format!("[\"{key}\",{index}]"), + "string", + value, + )); + } + } + Ok(()) +} + pub async fn permission_create( State(st): State>, Path(id): Path, Json(body): Json, ) -> Result, HttpError> { + if let Some(bad) = body.as_object().and_then(|fields| { + fields.keys().find(|key| { + ![ + "id", + "action", + "resources", + "save", + "metadata", + "source", + "agent", + ] + .contains(&key.as_str()) + }) + }) { + return Err(HttpError::TaggedData { + status: StatusCode::BAD_REQUEST, + tag: "InvalidRequestError", + fields: json!({"message": format!("unknown key {bad}")}), + }); + } let action = body.get("action").and_then(|v| v.as_str()); let resources = body.get("resources").and_then(|v| v.as_array()); let (Some(action), Some(resources)) = (action, resources) else { @@ -5554,8 +5611,9 @@ pub async fn permission_create( }; return Err(payload_err(key)); }; - if let Some(sv) = body.get("save").filter(|v| !v.is_null() && !v.is_array()) { - // probe-pinned bytes (save:false → freeze v2 envelope); null = absent + validate_permission_string_array("resources", resources)?; + if let Some(sv) = body.get("save").filter(|v| !v.is_array()) { + // `save` is an optional array in the frozen create schema; null is not absent. return Err(HttpError::TaggedData { status: StatusCode::BAD_REQUEST, tag: "InvalidRequestError", @@ -5565,6 +5623,48 @@ pub async fn permission_create( }), }); } + if let Some(save) = body.get("save").and_then(Value::as_array) { + validate_permission_string_array("save", save)?; + } + if let Some(metadata) = body.get("metadata") + && !metadata.is_object() + { + return Err(invalid_permission_field( + "[\"metadata\"]", + "object", + metadata, + )); + } + if let Some(source) = body.get("source") { + let valid = source.as_object().is_some_and(|fields| { + fields.len() == 3 + && fields.get("type").and_then(Value::as_str) == Some("tool") + && fields.get("messageID").is_some_and(Value::is_string) + && fields.get("id").is_some_and(Value::is_string) + }); + if !valid { + return Err(invalid_permission_field( + "[\"source\"]", + "Permission.Source", + source, + )); + } + } + if let Some(message) = body.get("message") + && !message.is_string() + { + return Err(invalid_permission_field("[\"message\"]", "string", message)); + } + if let Some(agent) = body.get("agent") + && !agent.is_null() + && !agent.is_string() + { + return Err(invalid_permission_field( + "[\"agent\"]", + "string | null", + agent, + )); + } // The v2 create IS the permission oracle: it returns the evaluated effect // (upstream PermissionV2.ask) with NO tool execution. It must evaluate the // session agent's ruleset — the previous hardcoded `effect:"allow"` was @@ -5598,18 +5698,23 @@ pub async fn permission_create( } let pid = format!("per_{}", ocserve_core::ids::ascending_tail()); // `ask` registers a real pending request (upstream `create`); other effects - // do not. The request body mirrors the v1 event shape so GET /permission - // and the reply routes see it. + // do not. Keep the gate value in v2 grammar; each HTTP family projects it + // at its own read boundary because the gate is shared with v1. if effect == "ask" { - let request = json!({ + let mut request = json!({ "id": pid, "sessionID": id, "action": action, "resources": resources, - "patterns": resources, - "always": resources, + "save": body.get("save").filter(|value| value.is_array()).cloned().unwrap_or(json!([])), "metadata": body.get("metadata").cloned().unwrap_or(json!({})), }); + if let Some(source) = body.get("source") { + request["source"] = source.clone(); + } + if let Some(message) = body.get("message").filter(|value| value.is_string()) { + request["message"] = message.clone(); + } st.gate.register_persistent(&pid, request); } Ok(Json(json!({"data": {"id": pid, "effect": effect}}))) diff --git a/crates/ocserve-http/tests/v2_surface.rs b/crates/ocserve-http/tests/v2_surface.rs index 971eb43..19cf3c1 100644 --- a/crates/ocserve-http/tests/v2_surface.rs +++ b/crates/ocserve-http/tests/v2_surface.rs @@ -12,17 +12,20 @@ use axum::body::Body; use axum::http::{Request, StatusCode}; use http_body_util::BodyExt; +use std::sync::Arc; use tower::ServiceExt; -fn app() -> axum::Router { +fn state_app() -> (Arc, axum::Router) { // Explicit v2=true: `AppState::new()` reads the process-global env, so // these tests only passed when the environment happened to set it (and a // gate-off run made the model-shape assertions reach the wrong parser). - ocserve_http::router(ocserve_http::AppState::with_payloads_and_v2( - None, - ocserve_http::Payloads::default(), - true, - )) + let state = + ocserve_http::AppState::with_payloads_and_v2(None, ocserve_http::Payloads::default(), true); + (state.clone(), ocserve_http::router(state)) +} + +fn app() -> axum::Router { + state_app().1 } async fn req( @@ -101,6 +104,114 @@ async fn session_diff_crash_path_returns_json_array() { assert!(v.is_array()); } +#[tokio::test] +async fn v2_permission_requests_project_v1_shape_for_reads() { + let (st, app) = state_app(); + st.gate.register_persistent( + "per_v1shape000000000000001", + serde_json::json!({ + "id": "per_v1shape000000000000001", + "sessionID": "ses_shape", + "permission": "bash", + "patterns": ["echo *"], + "always": ["echo *"], + "metadata": {"command": "echo hello"}, + "tool": {"messageID": "msg_tool", "callID": "call_tool"} + }), + ); + st.gate.register_persistent( + "per_other000000000000000001", + serde_json::json!({ + "id": "per_other000000000000000001", + "sessionID": "ses_other", + "permission": "read", + "patterns": ["/tmp/file"], + "always": [], + "metadata": {}, + "tool": {"messageID": "msg_other", "callID": "call_other"} + }), + ); + + let global = assert_json(&app, "GET", "/api/permission/request").await; + let global_request = global["data"] + .as_array() + .unwrap() + .iter() + .find(|request| request["id"] == "per_v1shape000000000000001") + .unwrap(); + assert_eq!(global_request["action"], "bash"); + assert_eq!(global_request["resources"], serde_json::json!(["echo *"])); + assert_eq!(global_request["save"], serde_json::json!(["echo *"])); + assert_eq!(global_request["source"]["type"], "tool"); + assert_eq!(global_request["source"]["messageID"], "msg_tool"); + assert_eq!(global_request["source"]["id"], "call_tool"); + assert!(global_request.get("permission").is_none()); + assert!(global_request.get("patterns").is_none()); + assert!(global_request.get("tool").is_none()); + + let session = assert_json(&app, "GET", "/api/session/ses_shape/permission").await; + assert_eq!(session["data"].as_array().unwrap().len(), 1); + assert_eq!(session["data"][0], *global_request); + + let detail = assert_json( + &app, + "GET", + "/api/session/ses_shape/permission/per_v1shape000000000000001", + ) + .await; + assert_eq!(detail["data"], *global_request); +} + +/// K-V2-PERM-SHAPE: a pending ask created through the v2 route must be +/// visible to BOTH grammars in the field names each frozen contract declares, +/// and the v2 source key is `id` (2.2.5), never `callID` (1.19.5). This drives +/// the real create handler so the gate stores a v2-shaped request, then reads +/// it back through both projectors; a hand-built gate entry would not prove +/// the create path. +#[tokio::test] +async fn v2_created_ask_is_readable_in_both_grammars() { + let (_st, app) = state_app(); + let sid = "ses_perm_both"; + let body = serde_json::json!({ + "action": "bash", + "resources": ["echo *"], + "save": ["echo *"], + "metadata": {"command": "echo hi"}, + "source": {"type": "tool", "messageID": "msg_src", "id": "call_src"}, + }); + let created = assert_json_post(&app, &format!("/api/session/{sid}/permission"), body).await; + assert_eq!(created["data"]["effect"], "ask", "{created}"); + let per_id = created["data"]["id"].as_str().expect("id").to_string(); + + let v2 = assert_json(&app, "GET", &format!("/api/session/{sid}/permission")).await; + let req = v2["data"] + .as_array() + .unwrap() + .iter() + .find(|r| r["id"] == per_id.as_str()) + .expect("v2 read finds the created ask"); + assert_eq!(req["action"], "bash"); + assert_eq!(req["resources"], serde_json::json!(["echo *"])); + assert_eq!(req["source"]["id"], "call_src"); + assert!( + req["source"].get("callID").is_none(), + "v2 source uses id, not callID" + ); + assert!(req.get("permission").is_none()); + + let v1 = assert_json(&app, "GET", "/permission").await; + let req1 = v1 + .as_array() + .unwrap() + .iter() + .find(|r| r["id"] == per_id.as_str()) + .expect("v1 read finds the same ask"); + assert_eq!(req1["permission"], "bash"); + assert_eq!(req1["patterns"], serde_json::json!(["echo *"])); + assert_eq!(req1["tool"]["callID"], "call_src"); + assert!(req1.get("action").is_none(), "v1 shape has no action key"); +} + #[tokio::test] async fn api_location_group_envelopes() { let app = app(); @@ -271,6 +382,7 @@ async fn permission_create_evaluates_ruleset() { agent: vec![ serde_json::json!({"name":"build","permission":[{"permission":"*","pattern":"*","action":"allow"}]}), serde_json::json!({"name":"locked","permission":[{"permission":"edit","pattern":"*","action":"deny"}]}), + serde_json::json!({"name":"asky","permission":[{"permission":"webfetch","pattern":"*","action":"ask"}]}), ], ..Payloads::default() }; @@ -285,7 +397,7 @@ async fn permission_create_evaluates_ruleset() { }, ); std::mem::forget(dir); - let app = ocserve_http::router(st); + let app = ocserve_http::router(st.clone()); // create a session let r = req(&app, "POST", "/session", Some(serde_json::json!({}))).await; @@ -317,31 +429,183 @@ async fn permission_create_evaluates_ruleset() { "a deny rule must yield deny, not the old hardcoded allow" ); - // ask registers a pending request visible on GET - let r = req( + // Ask stores v2 grammar in the shared gate; both read boundaries project + // to their frozen schemas, including the v1 UI's strict additionalProperties. + let response = req( &app, "POST", &format!("/api/session/{sid}/permission"), - Some(serde_json::json!({"action":"webfetch","resources":["x"],"save":[],"agent":"asky"})), + Some(serde_json::json!({ + "action":"webfetch", + "resources":["https://example.test"], + "save":["https://example.test/*"], + "metadata":{"url":"https://example.test"}, + "source":{"type":"tool","messageID":"msg_1","id":"call_1"}, + "agent":"asky" + })), ) .await; - // no "asky" agent → default build rules (allow) — use an unknown action - // under a build rule; instead assert unknown_tool is ask under locked - let r2 = req( + assert_eq!(response.status(), StatusCode::OK); + let permission_id = json(response).await["data"]["id"] + .as_str() + .unwrap() + .to_string(); + + let v2 = assert_json(&app, "GET", "/api/permission/request").await; + let v2_request = v2["data"] + .as_array() + .unwrap() + .iter() + .find(|request| request["id"] == permission_id) + .unwrap(); + assert_eq!(v2_request["action"], "webfetch"); + assert_eq!( + v2_request["resources"], + serde_json::json!(["https://example.test"]) + ); + assert_eq!( + v2_request["save"], + serde_json::json!(["https://example.test/*"]) + ); + + let v1 = assert_json(&app, "GET", "/permission").await; + let v1_request = v1 + .as_array() + .unwrap() + .iter() + .find(|request| request["id"] == permission_id) + .unwrap(); + assert_eq!(v1_request["permission"], "webfetch"); + assert_eq!( + v1_request["patterns"], + serde_json::json!(["https://example.test"]) + ); + assert_eq!( + v1_request["always"], + serde_json::json!(["https://example.test/*"]) + ); + assert_eq!(v1_request["tool"]["messageID"], "msg_1"); + assert_eq!(v1_request["tool"]["callID"], "call_1"); + for v2_only in ["action", "resources", "save", "source"] { + assert!(v1_request.get(v2_only).is_none(), "v1 leaked {v2_only}"); + } + + // V2 requests retain their wire grammar in the shared gate, but `always` + // must still create the grant and cascade to matching pending asks. + let sibling_response = req( &app, "POST", &format!("/api/session/{sid}/permission"), - Some(serde_json::json!({"action":"unknown_tool","resources":["*"],"save":[],"agent":"locked"})), + Some(serde_json::json!({ + "action":"webfetch", + "resources":["https://example.test/page"], + "save":["https://example.test/*"], + "agent":"asky" + })), + ) + .await; + assert_eq!(sibling_response.status(), StatusCode::OK); + let sibling_id = json(sibling_response).await["data"]["id"] + .as_str() + .unwrap() + .to_string(); + + let reply = req( + &app, + "POST", + &format!("/api/session/{sid}/permission/{permission_id}/reply"), + Some(serde_json::json!({"reply":"always"})), ) .await; - let _ = r; - // build has *:allow so unknown → allow; assert the shape still holds + assert_eq!(reply.status(), StatusCode::OK); + assert!( + st.gate + .check_always(&sid, "webfetch", "https://example.test/page") + ); + assert!( + !st.gate + .list() + .iter() + .any(|request| request["id"] == sibling_id), + "the matching v2 request should be resolved by the always grant" + ); +} + +#[tokio::test] +async fn permission_create_rejects_malformed_request_fields_before_persisting() { + use ocserve_http::{AppState, LlmRegistry, Payloads, Wires}; + let dir = tempfile::tempdir().unwrap(); + let db = dir.path().join("p.db"); + let blobs = + std::sync::Arc::new(ocserve_store::BlobStore::new(dir.path().join("blobs")).unwrap()); + let writer = std::sync::Arc::new(ocserve_store::Writer::spawn(db.clone()).unwrap()); + let llm = LlmRegistry { + modalities: Default::default(), + endpoints: Default::default(), + pricing: Default::default(), + limits: Default::default(), + default_model: ("p".into(), "m".into()), + systems: Default::default(), + default_agent: "build".into(), + }; + let payloads = Payloads { + agent: vec![serde_json::json!({ + "name":"asky", + "permission":[{"permission":"webfetch","pattern":"*","action":"ask"}] + })], + ..Payloads::default() + }; + let st = AppState::with_wiring( + None, + payloads, + Wires { + db, + blobs, + writer, + llm, + }, + ); + std::mem::forget(dir); + let app = ocserve_http::router(st); + let sid = new_session(&app).await; + let cases = [ + serde_json::json!({"action":"webfetch","resources":[7],"agent":"asky"}), + serde_json::json!({"action":"webfetch","resources":["https://example.test"],"save":[false],"agent":"asky"}), + serde_json::json!({"action":"webfetch","resources":["https://example.test"],"save":null,"agent":"asky"}), + serde_json::json!({"action":"webfetch","resources":["https://example.test"],"agent":7}), + serde_json::json!({"action":"webfetch","resources":["https://example.test"],"metadata":null,"agent":"asky"}), + serde_json::json!({"action":"webfetch","resources":["https://example.test"],"source":{"type":"user","messageID":"msg_1","id":"call_1"},"agent":"asky"}), + serde_json::json!({"action":"webfetch","resources":["https://example.test"],"source":{"type":"tool","messageID":"msg_1"},"agent":"asky"}), + serde_json::json!({"action":"webfetch","resources":["https://example.test"],"source":{"type":"tool","messageID":"msg_1","id":"call_1","extra":true},"agent":"asky"}), + serde_json::json!({"action":"webfetch","resources":["https://example.test"],"message":3,"agent":"asky"}), + serde_json::json!({"action":"webfetch","resources":["https://example.test"],"message":"extra field","agent":"asky"}), + ]; + let mut accepted_invalid_cases = Vec::new(); + for (index, body) in cases.into_iter().enumerate() { + let response = req( + &app, + "POST", + &format!("/api/session/{sid}/permission"), + Some(body), + ) + .await; + let status = response.status(); + let value = json(response).await; + if status != StatusCode::BAD_REQUEST { + accepted_invalid_cases.push((index, status, value["_tag"].clone())); + } else { + assert_eq!(value["_tag"], "InvalidRequestError"); + } + } assert!( - json(r2).await["data"]["id"] - .as_str() - .unwrap() - .starts_with("per") + accepted_invalid_cases.is_empty(), + "schema-invalid cases were accepted: {accepted_invalid_cases:?}" ); + let pending = assert_json(&app, "GET", "/api/permission/request").await; + assert_eq!(pending["data"], serde_json::json!([])); + // Negative control: without entry/value validation the malformed resource + // is skipped and answered allow; malformed optional fields can be stored + // by an ask rule and later violate Permission.Request's frozen schema. } /// 2026-10-11 bug hunt: v2 write routes must 404 an unknown session with the