diff --git a/approval-gate/Cargo.lock b/approval-gate/Cargo.lock index 6df09296b..471ea8705 100644 --- a/approval-gate/Cargo.lock +++ b/approval-gate/Cargo.lock @@ -2,6 +2,20 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "ahash" +version = "0.8.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" +dependencies = [ + "cfg-if", + "getrandom 0.3.4", + "once_cell", + "serde", + "version_check", + "zerocopy", +] + [[package]] name = "aho-corasick" version = "1.1.4" @@ -77,12 +91,13 @@ dependencies = [ "base64", "clap", "futures", + "harness", "iii-sdk", + "regex", "schemars", "serde", "serde_json", "serde_yaml", - "tempfile", "thiserror", "tokio", "tracing", @@ -107,12 +122,33 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" +[[package]] +name = "autocfg" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" + [[package]] name = "base64" version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "bit-set" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0700ddab506f33b20a03b13996eccd309a48e5ff77d0d95926aa0210fb4e95f1" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "349f9b6a179ed607305526ca489b34ad0a41aed5f7980fa90eb03160b69598fb" + [[package]] name = "bitflags" version = "2.13.0" @@ -128,12 +164,28 @@ dependencies = [ "generic-array", ] +[[package]] +name = "bstr" +version = "1.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63044e1ae8e69f3b5a92c736ca6269b8d12fa7efe39bf34ddb06d102cf0e2cab" +dependencies = [ + "memchr", + "serde", +] + [[package]] name = "bumpalo" version = "3.20.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" +[[package]] +name = "bytecount" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "175812e0be2bccb6abe50bb8d566126198344f707e304f45c648fd8f2cc0365e" + [[package]] name = "bytes" version = "1.11.1" @@ -249,6 +301,12 @@ version = "2.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4ae5f15dda3c708c0ade84bfee31ccab44a3da4f88015ed22f63732abe300c8" +[[package]] +name = "deranged" +version = "0.5.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cd812cc2bc1d69d4764bd80df88b4317eaef9e773c75226407d9bc0876b211c" + [[package]] name = "digest" version = "0.10.7" @@ -293,10 +351,15 @@ dependencies = [ ] [[package]] -name = "fastrand" -version = "2.4.1" +name = "fancy-regex" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6" +checksum = "531e46835a22af56d1e3b66f04844bed63158bc094a628bec1d321d9b4c44bf2" +dependencies = [ + "bit-set", + "regex-automata", + "regex-syntax", +] [[package]] name = "find-msvc-tools" @@ -319,6 +382,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fraction" +version = "0.15.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e076045bb43dac435333ed5f04caf35c7463631d0dae2deb2638d94dd0a5b872" +dependencies = [ + "lazy_static", + "num", +] + [[package]] name = "futures" version = "0.3.32" @@ -457,6 +530,40 @@ dependencies = [ "wasip3", ] +[[package]] +name = "globset" +version = "0.4.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "52dfc19153a48bde0cbd630453615c8151bce3a5adfac7a0aebfbf0a1e1f57e3" +dependencies = [ + "aho-corasick", + "bstr", + "log", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "harness" +version = "0.1.0" +dependencies = [ + "anyhow", + "async-trait", + "clap", + "globset", + "iii-sdk", + "jsonschema", + "schemars", + "serde", + "serde_json", + "serde_yaml", + "thiserror", + "tokio", + "tracing", + "tracing-subscriber", + "uuid", +] + [[package]] name = "hashbrown" version = "0.15.5" @@ -760,6 +867,15 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" +[[package]] +name = "iso8601" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1082f0c48f143442a1ac6122f67e360ceee130b967af4d50996e5154a45df46" +dependencies = [ + "nom", +] + [[package]] name = "itoa" version = "1.0.18" @@ -777,6 +893,34 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "jsonschema" +version = "0.18.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa0f4bea31643be4c6a678e9aa4ae44f0db9e5609d5ca9dc9083d06eb3e9a27a" +dependencies = [ + "ahash", + "anyhow", + "base64", + "bytecount", + "fancy-regex", + "fraction", + "getrandom 0.2.17", + "iso8601", + "itoa", + "memchr", + "num-cmp", + "once_cell", + "parking_lot", + "percent-encoding", + "regex", + "serde", + "serde_json", + "time", + "url", + "uuid", +] + [[package]] name = "lazy_static" version = "1.5.0" @@ -795,18 +939,21 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" -[[package]] -name = "linux-raw-sys" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" - [[package]] name = "litemap" version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92daf443525c4cce67b150400bc2316076100ce0b3686209eb8cf3c31612e6f0" +[[package]] +name = "lock_api" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "224399e74b87b5f3557511d98dff8b14089b3dadafcab6bb93eab67d3aace965" +dependencies = [ + "scopeguard", +] + [[package]] name = "log" version = "0.4.32" @@ -845,6 +992,15 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "nom" +version = "8.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df9761775871bdef83bee530e60050f7e54b1105350d6884eb0fb4f46c2f9405" +dependencies = [ + "memchr", +] + [[package]] name = "ntapi" version = "0.4.3" @@ -863,6 +1019,91 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "num" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" +dependencies = [ + "num-bigint", + "num-complex", + "num-integer", + "num-iter", + "num-rational", + "num-traits", +] + +[[package]] +name = "num-bigint" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a5e44f723f1133c9deac646763579fdb3ac745e418f2a7af9cd0c431da1f20b9" +dependencies = [ + "num-integer", + "num-traits", +] + +[[package]] +name = "num-cmp" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63335b2e2c34fae2fb0aa2cecfd9f0832a1e24b3b32ecec612c3426d46dc8aaa" + +[[package]] +name = "num-complex" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" +dependencies = [ + "num-traits", +] + +[[package]] +name = "num-conv" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "521739c6d2bac4aa25192232afe6841231376b2b26d4d9fae5ecf8ca5772e441" + +[[package]] +name = "num-integer" +version = "0.1.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7969661fd2958a5cb096e56c8e1ad0444ac2bbcd0061bd28660485a44879858f" +dependencies = [ + "num-traits", +] + +[[package]] +name = "num-iter" +version = "0.1.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1429034a0490724d0075ebb2bc9e875d6503c3cf69e235a8941aa757d83ef5bf" +dependencies = [ + "autocfg", + "num-integer", + "num-traits", +] + +[[package]] +name = "num-rational" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" +dependencies = [ + "num-bigint", + "num-integer", + "num-traits", +] + +[[package]] +name = "num-traits" +version = "0.2.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" +dependencies = [ + "autocfg", +] + [[package]] name = "objc2-core-foundation" version = "0.3.2" @@ -944,6 +1185,29 @@ dependencies = [ "tokio-stream", ] +[[package]] +name = "parking_lot" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93857453250e3077bd71ff98b6a65ea6621a19bb0f559a85248955ac12c45a1a" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2621685985a2ebf1c516881c026032ac7deafcda1a2c9b7850dc81e3dfcb64c1" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-link", +] + [[package]] name = "percent-encoding" version = "2.3.2" @@ -965,6 +1229,12 @@ dependencies = [ "zerovec", ] +[[package]] +name = "powerfmt" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" + [[package]] name = "ppv-lite86" version = "0.2.21" @@ -1098,6 +1368,27 @@ dependencies = [ "getrandom 0.3.4", ] +[[package]] +name = "redox_syscall" +version = "0.5.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" +dependencies = [ + "bitflags", +] + +[[package]] +name = "regex" +version = "1.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1292b7759ae1cb9ec195452d1390a074f0cd8541ab7a5a8c31cd6db45d4a6ba" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + [[package]] name = "regex-automata" version = "0.4.14" @@ -1173,19 +1464,6 @@ version = "2.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94300abf3f1ae2e2b8ffb7b58043de3d399c73fa6f4b73826402a5c457614dbe" -[[package]] -name = "rustix" -version = "1.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" -dependencies = [ - "bitflags", - "errno", - "libc", - "linux-raw-sys", - "windows-sys 0.61.2", -] - [[package]] name = "rustls" version = "0.23.40" @@ -1278,6 +1556,12 @@ dependencies = [ "syn", ] +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + [[package]] name = "security-framework" version = "3.7.0" @@ -1507,19 +1791,6 @@ dependencies = [ "windows", ] -[[package]] -name = "tempfile" -version = "3.27.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" -dependencies = [ - "fastrand", - "getrandom 0.4.2", - "once_cell", - "rustix", - "windows-sys 0.61.2", -] - [[package]] name = "thiserror" version = "2.0.18" @@ -1549,6 +1820,36 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "time" +version = "0.3.49" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "711a53c2d47bbd818258c498c8dbfe186a2526c631495cfe7e078567f86b8469" +dependencies = [ + "deranged", + "num-conv", + "powerfmt", + "serde_core", + "time-core", + "time-macros", +] + +[[package]] +name = "time-core" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e1c906769ad99c88eaa54e728060edef082f8e358ff32030cb7c7d315e81109" + +[[package]] +name = "time-macros" +version = "0.2.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "71c652a3727a9cbb9a02f707f530b618ce00d0ccd762009c8c23bd191df3c17d" +dependencies = [ + "num-conv", + "time-core", +] + [[package]] name = "tinystr" version = "0.8.3" diff --git a/approval-gate/Cargo.toml b/approval-gate/Cargo.toml index 913dbe5fa..4cd720fe6 100644 --- a/approval-gate/Cargo.toml +++ b/approval-gate/Cargo.toml @@ -31,6 +31,7 @@ async-trait = "0.1" # `RegisterTriggerType::trigger_request_format`. schemars = "0.8" base64 = "0.22" +regex = "1" [features] # Test-only seam: exposes `testkit` (engine bootstrap for the engine-backed @@ -45,4 +46,4 @@ testkit = [] approval-gate = { path = ".", features = ["testkit"] } futures = "0.3" uuid = { version = "1", features = ["v4"] } -tempfile = "3" +harness = { path = "../harness" } diff --git a/approval-gate/README.md b/approval-gate/README.md index 3b0ff41ff..5804d1783 100644 --- a/approval-gate/README.md +++ b/approval-gate/README.md @@ -6,8 +6,8 @@ worker: 1. **The gate** — `approval::gate`, a `pre_trigger` hook the worker binds itself at startup on the harness's `harness::hook::pre-trigger` trigger - type. It evaluates per-session mode, allow-lists, and the yaml policy, and - answers `continue`, `deny`, or `hold`. + type. It evaluates per-session mode, allow-lists, and inline config + `rules`, and answers `continue`, `deny`, or `hold`. 2. **The decision plane** — `approval::resolve` plus the per-session settings RPCs (`set_mode`, `add_always_allow`, `approve_always`, …). Human/console only. @@ -17,9 +17,10 @@ worker: notification workers and UIs bind to. The worker keeps **no resolved-approval history**: a record exists only while -a call is held; every record has an explicit deletion path and a cron sweep as -GC backstop. The transcript's `function_result` and the `pending_resolved` -event are the audit trail. +a call is held; every record has an explicit deletion path (resolve, turn +abort, session delete). The transcript's `function_result` and the +`pending_resolved` event are the audit trail. Holds do not expire — they wait +until a human resolves or the turn/session is purged. ## Standalone caveat @@ -40,10 +41,6 @@ exercises the harness surface against in-process fakes until harness 1.0 lands. iii worker add approval-gate ``` -The sweep needs the engine's cron worker: `iii worker add iii-cron`. Without -it the expiry backstop never fires (the harness pending sweep — once it -exists — remains the second backstop). - ## Quickstart ```bash @@ -76,22 +73,19 @@ unchanged from the proven implementation): 2. mode `full` → allow 3. `approved_always` hit → allow (**every** mode — remembered human decisions) 4. mode `auto` **and** `always_allow` hit → allow (dormant under `manual`) -5. fall through to `policy::check_permissions` (5s budget): - `allow` → allow · `deny` → deny · `needs_approval` → **hold** · - unparseable reply → hold · transport failure/timeout → **deny** - (`gate_unavailable` — fail closed, never an unattended hold) +5. fall through to configuration **`rules`** (first match wins): + `allow` → allow · `deny` → deny · no match → **hold** -No `policy::check_permissions` worker deployed? Every non-short-circuited -call is denied as `gate_unavailable`. Run a trivial policy worker (e.g. -"everything `needs_approval`") or lean on `always_allow_seed` / per-session -modes. +When the configuration entry omits `rules`, the gate denies only this +worker's own `approval::*` surface; every other call **holds**. On startup the +worker seeds/backfills the stored entry so `rules` are editable in the console. ## Custom trigger types | Type | Fires | Payload | |---|---|---| -| `approval::pending-created` | a call was held and its inbox record written (async, off the hot path) | `PendingApprovalRecord & { status: "pending" }` — redacted args, session context, expiry: self-sufficient for notification copy | -| `approval::pending-resolved` | a pending call left the inbox (exactly once per record) | ids + `outcome: "allow" \| "deny" \| "timeout" \| "aborted"`, operator `reason` on deny | +| `approval::pending-created` | a call was held and its inbox record written (async, off the hot path) | `PendingApprovalRecord & { status: "pending" }` — redacted args, session context: self-sufficient for notification copy | +| `approval::pending-resolved` | a pending call left the inbox (exactly once per record) | ids + `outcome: "allow" \| "deny" \| "aborted"`, operator `reason` on deny | Binding config (both types): `{ session_id?, metadata? }` — `metadata` is a subset-equality match against the record's denormalized `session_metadata`, @@ -103,24 +97,17 @@ restart, reconcile with one `approval::list-pending` call. The whole config — runtime wiring **and** deployment approval defaults — lives in the single engine configuration entry **`approval-gate`** (operator-edited via the console's Configuration screen; reactive reload, no polling). There is -**no committed `config.yaml`**; defaults are seeded into the entry on first -registration. +**no committed `config.yaml`**. On first boot the worker seeds the entry with +the built-in defaults (including `rules`) so the editor is pre-filled; existing +stored values are never overwritten except to add a missing `rules` field. ```jsonc { - "hook": { // harness::hook::pre-trigger binding (re-bound live on change) - "functions": ["*"], // pre_trigger globs the gate consults on - "timeout_ms": 5000, - "on_error": "fail_closed" - }, - "sweep_expression": "0 * * * * *", // 6-field cron for the expiry sweep (re-bound live on change) - "policy_timeout_ms": 5000, // per-call budgets (hot-reloadable) - "session_fetch_timeout_ms": 1000, - "state_timeout_ms": 5000, - "harness_timeout_ms": 10000, "default_mode": "manual", // manual | auto | full — sessions with no stored settings - "always_allow_seed": [], // auto-mode trust profile (function ids / globs) - "pending_timeout_ms": 1800000 // hold deadline; drives expires_at (default 30 min) + "rules": [ // first match wins; no match → hold + "!approval::*", + { "function": "state::get", "action": "allow", "modes": ["auto"] } + ] } ``` @@ -129,9 +116,7 @@ schema and fetches the authoritative value at startup, and a failed register/fetch aborts boot (the gate must run on a known, authoritative policy surface, never a guessed one). When no value is stored yet the built-in defaults above are seeded and used. **Every field hot-reloads on -`configuration::set` — nothing requires a restart**: `hook` and -`sweep_expression` re-bind their triggers live (register the new binding, then -unregister the old); the rest swap the in-memory snapshot. +`configuration::set` — nothing requires a restart**. ## Agent exposure @@ -154,8 +139,9 @@ cargo clippy --all-targets --all-features -- -D warnings The integration suite spawns a real engine (`III_ENGINE_BIN` or `iii` on PATH) with `configuration` + `iii-state`, registers the production surface -in-process, and fakes the not-yet-built siblings -(`policy::check_permissions`, `harness::function::resolve`, `session::get`). +in-process, and fakes sibling RPCs where noted (`session::get`). With the +harness binary available, `tests/harness_integration.rs` additionally boots the +real harness worker for cross-worker hold / sweep / resolve checks. ## Architecture documentation diff --git a/approval-gate/architecture/README.md b/approval-gate/architecture/README.md index 7a6f91f1b..cee26f015 100644 --- a/approval-gate/architecture/README.md +++ b/approval-gate/architecture/README.md @@ -13,6 +13,7 @@ opening the source. |---|---|---| | [internals.md](internals.md) | Maintainers of this worker | You are changing approval-gate itself: the evaluation order, the pending-record lifecycle, the emit gate, redaction, configuration reload. | | [integration.md](integration.md) | Authors of other workers / clients | You are building something that calls `approval::*` or binds its trigger types — the console, a notification worker, the harness (once its hook surface lands). This file is the handoff contract. | +| [permissions-source.md](permissions-source.md) | Operators / integrators | You need to know where permission truth lives and how harness and the console consume the single `approval-gate` rules list. | The unit suites beside each module and the engine-backed scenarios in [../tests/integration.rs](../tests/integration.rs) are the executable @@ -24,17 +25,17 @@ rows, and the exactly-once emission contract are all pinned by tests. approval-gate decides, for one function call at a time, whether a human must be involved — and routes the human's answer back to the parked turn. It is a `pre_trigger` hook (`approval::gate`) that answers `continue` / `deny` / -`hold` from a per-session permission model (mode + two allow-lists) with a -yaml-policy fallback; a decision plane (`approval::resolve` + settings RPCs, +`hold` from a per-session permission model (mode + two allow-lists) with an +inline config-`rules` fallback; a decision plane (`approval::resolve` + settings RPCs, human/console-only); and an **ephemeral** pending inbox (state scope `approval_pending`, two custom trigger types) that exists only while calls are held. It never executes the held function itself — on allow it asks the harness to release the call through its own trigger pipeline -(`harness::function::resolve`, `action: "execute"`); on deny/timeout it +(`harness::function::resolve`, `action: "execute"`); on deny it delivers an `is_error` result. No decision history is kept: the transcript and the `pending_resolved` event are the audit trail, and every state record -this worker writes has an explicit deletion path plus a cron sweep as GC -backstop. +this worker writes has an explicit deletion path (resolve, turn abort, +session delete). Holds do not expire. ```mermaid flowchart LR @@ -47,7 +48,6 @@ flowchart LR R -- "delete (emit gate)" --> S R -. "pending_resolved" .-> N C[(configuration entry\napproval-gate)] -. "reactive reload" .-> G - CR[cron ~60s] --> SW[approval::sweep] --> S ``` ## Vocabulary diff --git a/approval-gate/architecture/integration.md b/approval-gate/architecture/integration.md index 6d38c0305..d1bb5fbfe 100644 --- a/approval-gate/architecture/integration.md +++ b/approval-gate/architecture/integration.md @@ -21,7 +21,7 @@ operational contract). | `approval::approve-always` | console (human-only) | Per-session grant honoured in **every** mode; call it right before `resolve { decision: "allow" }` for an "Approve always" button. | | `approval::get-settings` | console | Effective settings + `source: "stored" \| "defaults"`. Never writes. | | `approval::clear-settings` | console | Drop the stored record; revert to deployment defaults. | -| `approval::on-config-change` / `on-session-deleted` / `on-turn-completed` / `approval::sweep` | trigger handlers | Internal — never call directly. | +| `approval::on-config-change` / `on-session-deleted` / `on-turn-completed` | trigger handlers | Internal — never call directly. | Errors use `code: message` with codes `approval/invalid_payload`, `approval/state_unavailable`, `approval/harness_unavailable`. An unknown @@ -43,7 +43,7 @@ hook returns `hold` — never on the trigger hot path. Payload: the `PendingApprovalRecord` plus `status: "pending"` — ids (`session_id`, `turn_id`, `function_call_id`, `function_id`), redacted -`arguments_excerpt`, `pending_at` / `expires_at`, denormalized +`arguments_excerpt`, `pending_at`, denormalized `session_title` / `session_description` / `session_metadata` (omitted when session-manager was unreachable at hold time), sub-agent `depth`. Self-sufficient for notification copy — no follow-up reads needed, and safe @@ -53,7 +53,7 @@ to forward to push/Slack payloads (arguments are redacted and clipped). A pending call left the inbox. Emitted **exactly once per record** — your badge-clearing logic can trust it. Payload: ids plus -`outcome: "allow" | "deny" | "timeout" | "aborted"`, operator `reason` (deny +`outcome: "allow" | "deny" | "aborted"`, operator `reason` (deny only), `session_metadata`, `resolved_at`. ### Binding config (both types) @@ -76,7 +76,7 @@ sequenceDiagram participant UI as console participant N as notify worker H->>AG: approval::gate (pre_trigger hook) - AG-->>H: { decision: "hold", pending_timeout_ms } + AG-->>H: { decision: "hold", pending_timeout_ms: 0 } AG--)N: approval::pending-created UI->>AG: approval::resolve { decision: "allow" } AG->>H: harness::function::resolve { action: "execute" } @@ -96,8 +96,8 @@ assumes, faked today by `tests/integration.rs`: - **`harness::hook::pre-trigger` trigger type.** The worker binds `approval::gate` at startup with - `{ functions, timeout_ms, on_error: "fail_closed" }` from the `hook` block of - its `approval-gate` configuration entry. The hook is an ordinary registered + `{ functions: ["*"], timeout_ms: 5000, on_error: "fail_closed" }` at worker + startup (fixed — not in the configuration entry). The hook is an ordinary registered function: the harness invokes it synchronously and treats the return value as `HookOutput`. - **`harness::function::resolve`** accepting @@ -116,24 +116,15 @@ gated sets a broad trigger policy and lets the gate hold/deny. ## Deployment notes -- **Sweep requires `iii-cron`** (`iii worker add iii-cron`). The binding - config key is `expression` (6-field cron, default `"0 * * * * *"`). -- **Policy worker** (`policy::check_permissions`): soft dependency with a - sharp consequence — absent, every non-short-circuited call denies as - `gate_unavailable`. Deploy a trivial "everything needs_approval" policy - worker or rely on modes/allow-lists. - **session-manager** (soft): provides hold-time context and the `session::deleted` cascade. Without it, records carry no session context and settings cleanup relies on `approval::clear-settings`. -- **Configuration (required)**: the worker's entire config — the `hook` - binding, `sweep_expression`, the per-call `*_timeout_ms` budgets, and the - approval defaults (`default_mode`, `always_allow_seed`, `pending_timeout_ms`) - — lives in the `approval-gate` configuration entry; there is **no +- **Configuration (required)**: the worker's config — the approval defaults + (`default_mode`, `rules`) — lives in the `approval-gate` configuration entry; there is **no `config.yaml`**. It is a required boot dependency: a failed register/fetch aborts startup. `configuration::set` replaces the **whole** value — - read-merge-write to edit one field. Every field hot-reloads (no restart): - `hook` and `sweep_expression` re-bind their triggers live; the rest swap the - in-memory snapshot. + read-merge-write to edit one field. When `rules` is omitted, the built-in + shipped defaults apply. Every field hot-reloads via snapshot swap. ## What not to do diff --git a/approval-gate/architecture/internals.md b/approval-gate/architecture/internals.md index db22301a4..be3beed7d 100644 --- a/approval-gate/architecture/internals.md +++ b/approval-gate/architecture/internals.md @@ -11,16 +11,16 @@ For maintainers changing this worker. The integration contract lives in | `types.rs` | Every wire type (serde + schemars), id validation (`/` is the reserved key separator), `metadata_matches` subset-equality. | | `state.rs` / `harness.rs` / `session.rs` | Thin per-target `iii.trigger` wrappers (state kv, `harness::function::resolve`, `session::get`). No transport abstraction; tests run against a real engine via `testkit/engine.rs`. | | `decision.rs` | **Pure** evaluation order (no I/O): human-only prefix check, mode/allow-list short-circuits, `*`-glob matching. | -| `policy.rs` | `policy::check_permissions` client: 5s budget, reply parsing, failure mapping. | +| `permissions/` | Inline rule compiler + evaluator (first match wins; no match → hold). Built-in default: deny `approval::*` only. | | `denial.rs` | `DenialEnvelope` assembly + text rendering; reason strings ported verbatim from the prior art. | | `redact.rs` | Recursive argument redaction (pure port of the proven `redact.ts`). | | `settings.rs` | Effective-settings computation, lazy seeding, immutable mutation helpers, tolerant vs strict reads. | | `pending.rs` | Inbox record store: `get`/`put`/`list_all` and **`delete_with_gate`** — the single deletion helper. | -| `config.rs` | The single `WorkerConfig` (Path B): serde + schemars schema, `from_yaml`/`from_json`/`to_json`/`json_schema`/`boot_signature` (`hook` + `sweep_expression` are the structural fields — re-bound live on change). | +| `config.rs` | The single `WorkerConfig` (Path B): serde + schemars schema, `from_yaml`/`from_json`/`to_json`/`json_schema` and `permissions()` (compiles the inline `rules`). | | `configuration.rs` | `configuration` worker integration: `register_config` / `fetch_config`, the `ConfigCell` snapshot, `reloadable`, and the typed, re-fetching `approval::on-config-change` trigger handler. | | `events.rs` | The two custom trigger types, `SubscriberSet`s, binding filters, the `EventSink` trait + `Emitter` (Void-action fan-out). | | `functions/` | One file per `approval::*` function; `mod.rs` holds `Deps` and the typed registration helper. | -| `main.rs` | Boot order: register+fetch config (fatal) → trigger types → functions → best-effort hook/cron/session bindings → `register_config_trigger` (last). | +| `main.rs` | Boot order: register+fetch config (fatal) → trigger types → functions → best-effort hook/session bindings → `register_config_trigger` (last). | Every handler takes `Deps { iii, sink, config }` — a `ConfigCell` it snapshots once per call via `deps.config().await` — and reaches siblings @@ -49,15 +49,13 @@ adapt to). `approved_always` hit → continue (every mode); `auto` + `always_allow` hit → continue. Allow-list entries match by equality fast-path or `*` glob (seed entries are documented as "ids / globs"). -5. Policy fallback: `allow` → continue; `deny` → deny with the permissions - envelope reason; **unparseable reply → hold** (a human look is the safe - reading of "don't know"); **transport failure / timeout → deny** - (`gate_unavailable` — never an unattended hold). +5. Config rules fallback: `allow` → continue; `deny` → deny with the permissions + envelope reason; no match → **hold**. 6. Hold path: - Idempotency first: an existing record (redelivered at-least-once step) - returns `hold` without rewriting or re-emitting. - - `session::get` soft-fetch under its own `session_fetch_timeout_ms` - budget; context fields are omitted on any failure. + returns `hold` with `pending_timeout_ms: 0` without rewriting or re-emitting. + - `session::get` soft-fetch under a fixed 1s budget; context fields are + omitted on any failure. - Record written **synchronously before returning hold** — write failure → deny (`gate_unavailable`), never hold blind. A non-null `old_value` on the write means a concurrent duplicate won the race: skip emission. @@ -70,10 +68,10 @@ Two scopes, both with explicit deletion paths: | Scope/key | Created | Deleted by | |---|---|---| -| `approval_pending//` | in-hook, before `hold` returns | resolve · `harness::turn-completed` · `session::deleted` · sweep on `expires_at` | +| `approval_pending//` | in-hook, before `hold` returns | resolve · `harness::turn-completed` · `session::deleted` | | `approval_settings/` | first user mutation (lazy; reads never write) | `session::deleted` · `approval::clear-settings` | -All four pending-deletion paths funnel through +All three pending-deletion paths funnel through `pending::delete_with_gate`, which is where exactly-once emission is decided: 1. `state::set { value: null }` — the engine swaps the value under its write @@ -92,10 +90,7 @@ live value and double-emit. Crash ordering in resolve: `harness::function::resolve` **first**, then delete, then emit. A crash between resolve and delete leaks one record until -the sweep collects it; a decision is never lost. The sweep tolerates -`{ resolved: false }` and transport errors from the harness and deletes the -expired record regardless — the inbox must stay O(live holds) even in a -deployment with no harness at all. +turn/session cleanup collects it; a decision is never lost. ## Settings: lazy seeding @@ -121,27 +116,19 @@ mirroring `context-manager` / `session-manager`. `register_config` registers the `WorkerConfig` JSON Schema and seeds `WorkerConfig::default()` as `initial_value` only when no value is stored yet (re-registration preserves the stored value); `fetch_config` reads the authoritative, env-expanded value at -boot. Both are **required** — a failed register/fetch aborts boot, so the gate -always runs on a known, authoritative policy surface (never a guessed one). -When nothing is stored, the built-in defaults (`manual`, `[]`, 30 min, the `*` -hook, the per-minute sweep) are what gets seeded and used. +boot. The register/fetch pair is **required** — a failed register/fetch aborts +boot. When nothing is stored, the built-in defaults are seeded and used. The live value is held in a `ConfigCell` (`Arc>>`) that every handler snapshots per call. `register_config_trigger` registers the -**typed** `approval::on-config-change` handler (`OnConfigChangeEvent` → -`OnConfigChangeResponse` — never a `Value` handler, registered off the public -`catalog()`) and binds the `configuration` trigger. On `configuration:updated` -it **re-fetches** via `configuration::get` (ignoring the trigger payload, so a -direct call can't inject config) and swaps the cell. A change to the boot -signature (`hook` or `sweep_expression`) **re-binds** the affected trigger live: -`register_config_trigger` retains the `Trigger` handles in `TriggerHandles`, and -the handler registers the new binding then `unregister()`s the old (a fail-safe -overlap — the gate is idempotent, so a brief double-fire is harmless), so no -field requires a restart; every other field hot-applies. The config parse is **strict** -(`deny_unknown_fields`): an unparseable stored value is rejected and the -last-good snapshot kept, so a typo'd operator edit can't silently widen -access. (Per-session settings records keep their own tolerant read — see -*Settings: lazy seeding*.) +**typed** `approval::on-config-change` handler and binds the `configuration` +trigger. On `configuration:updated` it **re-fetches** via `configuration::get` +(ignoring the trigger payload) and swaps the cell. The harness `pre_trigger` +hook binding is fixed at worker startup (`["*"]`, 5s, fail-closed). The config +parse is **strict** (`deny_unknown_fields`): an unparseable stored value is +rejected and the last-good snapshot kept, so a typo'd operator edit can't +silently widen access. (Per-session settings records keep their own tolerant +read — see *Settings: lazy seeding*.) ## Redaction (`redact.rs`) @@ -161,8 +148,6 @@ Verified against the engine source (`~/workspaces/personal/motia/iii`): is **stored**, not a delete (hence the two-step delete above). - `state::get` / `state::delete` return the (old) value or null; `state::list` returns the scope's **values only**, no keys, no pagination. -- The cron trigger's config key is **`expression`** (6-field cron), not - `schedule` — `docs/sops/binary-worker.md` is stale on this. - `register_trigger` acks asynchronously: `Ok` means "request sent"; a missing trigger type surfaces later as an SDK-level `trigger_type_not_found` error log. Boot therefore never depends on @@ -171,7 +156,7 @@ Verified against the engine source (`~/workspaces/personal/motia/iii`): ## Testing - **Unit** (`cargo test`): every module has a suite beside it. Pure-logic - modules (`decision`, `redact`, `denial`, `settings`, `types`, …) run with + modules (`decision`, `redact`, `denial`, `settings`, `permissions`, `types`, …) run with no engine. The `approval::*` handlers run against a real spawned engine via `testkit::engine` (`III_ENGINE_BIN` or `iii` on PATH; self-skips otherwise), so `delete_with_gate`'s null-tombstone invariant is exercised diff --git a/approval-gate/architecture/permissions-source.md b/approval-gate/architecture/permissions-source.md new file mode 100644 index 000000000..574771853 --- /dev/null +++ b/approval-gate/architecture/permissions-source.md @@ -0,0 +1,26 @@ +# Permission sources (inventory) + +Before the single-source consolidation, four places could disagree about +what an agent may do: + +| Source | Location | What it holds | +|---|---|---| +| **Deployment rules** | `configuration` entry `approval-gate` → `rules` (+ legacy `always_allow_seed`, now folded into mode-scoped allow rules) | First-match `allow` / `deny` / `hold` for every function call | +| **Per-session deltas** | `state` scope `approval_settings/` → `mode`, `always_allow`, `approved_always` | Human choices layered on top of deployment defaults | +| **Console defaults (removed)** | ~~`localStorage` `iii-default-permission-mode` / `iii-default-allowlist`~~ | Was a fifth copy; console now reads/writes the `approval-gate` entry | +| **Harness structural floor** | Per-turn `FunctionPolicy { allow, deny, expose }` on `harness::send` | Fail-closed globs before the pre_trigger hook chain; derived from deployment rules at send time | + +## Canonical source + +**`approval-gate` configuration `rules`** is the single deployment policy. +Auto-mode trust that used to live in `always_allow_seed` is expressed as +`allow` rules with `"modes": ["auto"]`. Session `always_allow` / +`approved_always` remain per-session human deltas. + +## Consumers + +| Player | How it implements the one source | +|---|---| +| **approval-gate** | Evaluates `rules` (+ session deltas) in `approval::gate` | +| **Harness** | `FunctionPolicy` on send derived from the same rules; `approval::gate` bound as `pre_trigger` | +| **Console** | Edits `default_mode` + auto allowlist via `configuration::get/set` on `approval-gate` | diff --git a/approval-gate/src/config.rs b/approval-gate/src/config.rs index 7f735e360..08a6025a0 100644 --- a/approval-gate/src/config.rs +++ b/approval-gate/src/config.rs @@ -9,126 +9,64 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use serde_json::Value; +use serde_json::{json, Value}; +use crate::permissions::{default_rule_specs, parse_rules_from_config, Permissions, RuleSpec}; use crate::types::PermissionMode; -fn default_hook_functions() -> Vec { - vec!["*".to_string()] +/// The shipped permission rules as a JSON array of shorthand strings — the +/// serde default for [`WorkerConfig::rules`] and the value seeded into the +/// configuration entry on first boot. Sourced from +/// [`crate::permissions::default_rule_specs`] so the in-memory defaults and +/// the console-editable list never drift. +pub fn default_rules_value() -> Vec { + default_rule_specs() + .into_iter() + .filter_map(|r| match r { + RuleSpec::Shorthand(s) => Some(Value::String(s)), + _ => None, + }) + .collect() } -fn default_hook_timeout_ms() -> u64 { - 5_000 +fn default_rules() -> Vec { + default_rules_value() } -fn default_on_error() -> String { - "fail_closed".to_string() +/// JSON Schema for the `rules` array — string shorthands the console +/// renders as an editable list. Matches what the gate actually evaluates: +/// `parse_rules_from_config` reads each entry as a shorthand. +fn rules_schema(_gen: &mut schemars::r#gen::SchemaGenerator) -> schemars::schema::Schema { + serde_json::from_value(json!({ + "type": "array", + "description": "Permission rules for the gate hook (first match wins). Each entry is a string or object.\n\nString shorthands: bare id/glob → allow; prefix ! → deny; no match → hold.\n\nStructured objects: { \"function\": \"shell::*\", \"action\": \"allow\" | \"deny\", \"modes\": [\"auto\"] } — optional modes scope the rule to manual, auto, or full (omit for all modes). Use modes: [\"auto\"] on allow rules to seed the auto-mode trust list.\n\nExamples:\n• \"state::get\" — allow reads\n• \"shell::*\" — allow any shell worker call\n• \"!approval::*\" — deny the approval decision plane (shipped default)\n• { \"function\": \"web::fetch\", \"action\": \"allow\", \"modes\": [\"auto\"] } — auto-mode trust", + "items": { + "type": "string", + "description": "Function id or glob. Allow: \"web::fetch\", \"coder::*\". Deny: \"!configuration::*\", \"!router::chat\"." + }, + "default": default_rules_value(), + })) + .expect("rules JSON Schema is valid") } -/// The `harness::hook::pre-trigger` binding the worker registers for -/// itself at startup. Consumed once at boot (part of the boot signature); -/// a live change requires a restart to re-bind the hook. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] -#[serde(deny_unknown_fields)] -pub struct HookBinding { - /// pre_trigger target globs to consult on; omit-equivalent default - /// (`["*"]`) consults on every call. - #[serde(default = "default_hook_functions")] - pub functions: Vec, - #[serde(default = "default_hook_timeout_ms")] - pub timeout_ms: u64, - /// `fail_closed` is already the harness `pre_*` default — a crashed - /// gate must deny, not wave calls through. - #[serde(default = "default_on_error")] - pub on_error: String, -} - -impl Default for HookBinding { - fn default() -> Self { - Self { - functions: default_hook_functions(), - timeout_ms: default_hook_timeout_ms(), - on_error: default_on_error(), - } - } -} - -fn default_sweep_expression() -> String { - // 6-field cron (engine cron worker, config key "expression"): once - // daily at midnight. - "0 0 0 * * *".to_string() -} - -fn default_policy_timeout_ms() -> u64 { - 5_000 -} - -fn default_session_fetch_timeout_ms() -> u64 { - 1_000 -} - -fn default_state_timeout_ms() -> u64 { - 5_000 -} - -fn default_harness_timeout_ms() -> u64 { - 10_000 -} - -fn default_pending_timeout_ms() -> i64 { - 1_800_000 -} - -/// The worker's single configuration entry — runtime wiring AND deployment -/// approval defaults in one schema-validated value. Split on a live update -/// into: -/// -/// - The BOOT SIGNATURE (`hook` + `sweep_expression`): consumed ONCE at -/// startup to bind the `harness::hook::pre-trigger` hook and the cron -/// sweep. A config change that alters either is REFUSED on hot-reload -/// (logged "restart required", the previous snapshot kept). -/// - Every OTHER field is a per-call tuning knob (the `*_timeout_ms` -/// budgets and the approval defaults `default_mode` / `always_allow_seed` -/// / `pending_timeout_ms`). When a freshly-fetched config's boot -/// signature matches, the snapshot is swapped live; handlers read the -/// current snapshot per call via [`Deps::config`](crate::functions::Deps::config). +/// The worker's single configuration entry — deployment approval defaults. +/// Every field hot-reloads via [`crate::configuration`]; handlers read the +/// live snapshot per call through [`Deps::config`](crate::functions::Deps::config). #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] pub struct WorkerConfig { - /// The `harness::hook::pre-trigger` binding (restart-required). - #[serde(default)] - pub hook: HookBinding, - /// 6-field cron expression for the expiry sweep (restart-required). - #[serde(default = "default_sweep_expression")] - pub sweep_expression: String, - - /// Fail-closed budget for the synchronous `policy::check_permissions` - /// consult. - #[serde(default = "default_policy_timeout_ms")] - pub policy_timeout_ms: u64, - /// Best-effort `session::get` budget inside the hook (record context - /// fields are omitted when this is exceeded). - #[serde(default = "default_session_fetch_timeout_ms")] - pub session_fetch_timeout_ms: u64, - /// Budget for `state::*` calls. - #[serde(default = "default_state_timeout_ms")] - pub state_timeout_ms: u64, - /// Budget for `harness::function::resolve` calls. - #[serde(default = "default_harness_timeout_ms")] - pub harness_timeout_ms: u64, - /// Effective permission mode for sessions with no stored approval /// settings record. #[serde(default)] pub default_mode: PermissionMode, - /// Deployment trust profile for auto mode (function ids / globs); - /// copied into a session's settings on its first mutation. - #[serde(default)] - pub always_allow_seed: Vec, - /// Hold deadline in milliseconds; drives `expires_at` on pending - /// records. - #[serde(default = "default_pending_timeout_ms")] - pub pending_timeout_ms: i64, + /// Agent permission rules evaluated inline by the gate (first match + /// wins; no match → the call is held for human approval). String + /// shorthands: bare id/glob → allow, `!`-prefixed → deny. Structured + /// objects may set `"modes": ["auto"]` on allow rules to seed the + /// per-session auto-mode trust list. + #[serde(default = "default_rules")] + #[schemars(schema_with = "rules_schema")] + pub rules: Vec, } impl WorkerConfig { @@ -159,7 +97,25 @@ impl WorkerConfig { serde_json::to_value(self).expect("WorkerConfig serializes") } - /// The JSON Schema registered with the `configuration` worker. Field + /// Compile the configured `rules` into the inline permission matcher. + /// Tolerant: invalid entries are skipped and an empty list yields a + /// matcher that holds every call (fail-closed, never fail-open). + pub fn permissions(&self) -> Permissions { + let specs = parse_rules_from_config(&Value::Array(self.rules.clone())); + Permissions::compile_tolerant(&specs) + } + + /// Rule specs parsed from the configured `rules` array. + pub fn rule_specs(&self) -> Vec { + parse_rules_from_config(&Value::Array(self.rules.clone())) + } + + /// Auto-mode trust globs derived from allow rules scoped to `auto`. + pub fn auto_allow_seed(&self) -> Vec { + crate::permissions::auto_allow_seed_from_specs(&self.rule_specs()) + } + + /// The JSON Schema registered with the `configuration` worker. /// doc-comments become property descriptions; the shipped defaults /// are attached as a top-level `example`. pub fn json_schema() -> Value { @@ -177,43 +133,13 @@ impl WorkerConfig { } schema } - - /// The **structural** fields — the two trigger bindings. `hook` backs - /// the `harness::hook::pre-trigger` binding and `sweep_expression` the - /// cron sweep; a live change re-binds the affected trigger on the fly - /// (register the new binding, then unregister the old — see - /// [`crate::configuration`]), so neither needs a restart. Every other - /// field is a per-call tuning knob read from the live snapshot. - pub fn boot_signature(&self) -> BootSignature { - BootSignature { - hook: self.hook.clone(), - sweep_expression: self.sweep_expression.clone(), - } - } -} - -/// Signature of the structural config fields — the two trigger bindings -/// (see [`WorkerConfig::boot_signature`]). An equal signature means only -/// per-call knobs changed (swap the snapshot); a different signature -/// re-binds the changed trigger(s) live. -#[derive(Clone, PartialEq, Eq, Debug)] -pub struct BootSignature { - pub hook: HookBinding, - pub sweep_expression: String, } impl Default for WorkerConfig { fn default() -> Self { Self { - hook: HookBinding::default(), - sweep_expression: default_sweep_expression(), - policy_timeout_ms: default_policy_timeout_ms(), - session_fetch_timeout_ms: default_session_fetch_timeout_ms(), - state_timeout_ms: default_state_timeout_ms(), - harness_timeout_ms: default_harness_timeout_ms(), default_mode: PermissionMode::default(), - always_allow_seed: Vec::new(), - pending_timeout_ms: default_pending_timeout_ms(), + rules: default_rules(), } } } @@ -254,17 +180,9 @@ mod tests { #[test] fn defaults_match_the_spec_wiring() { let cfg = WorkerConfig::default(); - assert_eq!(cfg.hook.functions, vec!["*".to_string()]); - assert_eq!(cfg.hook.timeout_ms, 5_000); - assert_eq!(cfg.hook.on_error, "fail_closed"); - assert_eq!(cfg.sweep_expression, "0 0 0 * * *"); - assert_eq!(cfg.policy_timeout_ms, 5_000); - assert_eq!(cfg.session_fetch_timeout_ms, 1_000); - assert_eq!(cfg.state_timeout_ms, 5_000); - assert_eq!(cfg.harness_timeout_ms, 10_000); assert_eq!(cfg.default_mode, PermissionMode::Manual); - assert!(cfg.always_allow_seed.is_empty()); - assert_eq!(cfg.pending_timeout_ms, 1_800_000); + assert!(!cfg.rules.is_empty()); + assert!(cfg.auto_allow_seed().is_empty()); } #[test] @@ -275,11 +193,8 @@ mod tests { #[test] fn partial_yaml_fills_defaults() { - let cfg: WorkerConfig = - serde_yaml::from_str("hook:\n functions: [\"shell::*\"]\n").unwrap(); - assert_eq!(cfg.hook.functions, vec!["shell::*".to_string()]); - assert_eq!(cfg.hook.timeout_ms, 5_000); - assert_eq!(cfg.default_mode, PermissionMode::Manual); + let cfg: WorkerConfig = serde_yaml::from_str("default_mode: auto\n").unwrap(); + assert_eq!(cfg.default_mode, PermissionMode::Auto); } #[test] @@ -295,17 +210,7 @@ mod tests { .get("properties") .and_then(|p| p.as_object()) .expect("schema has a properties object"); - for field in [ - "hook", - "sweep_expression", - "policy_timeout_ms", - "session_fetch_timeout_ms", - "state_timeout_ms", - "harness_timeout_ms", - "default_mode", - "always_allow_seed", - "pending_timeout_ms", - ] { + for field in ["default_mode", "rules"] { assert!( props.get(field).is_some(), "missing schema property {field}" @@ -332,64 +237,42 @@ mod tests { } #[test] - fn from_json_round_trips_custom_values() { - let json = serde_json::json!({ - "default_mode": "auto", - "always_allow_seed": ["state::get"], - "pending_timeout_ms": 60_000, - }); - let cfg = WorkerConfig::from_json(&json).unwrap(); - assert_eq!(cfg.default_mode, PermissionMode::Auto); - assert_eq!(cfg.always_allow_seed, vec!["state::get".to_string()]); - assert_eq!(cfg.pending_timeout_ms, 60_000); - // Unspecified fields fall back to serde defaults. - assert_eq!(cfg.state_timeout_ms, 5_000); + fn from_json_rejects_removed_fields() { + let err = WorkerConfig::from_json(&serde_json::json!({ + "hook": { "functions": ["*"] } + })) + .unwrap_err(); + assert!(err.contains("json parse"), "got: {err}"); } #[test] - fn from_json_rejects_garbage() { - let err = WorkerConfig::from_json(&serde_json::json!({ "default_mode": 42 })).unwrap_err(); - assert!(err.contains("json parse"), "got: {err}"); - let err = WorkerConfig::from_json(&serde_json::json!("garbage")).unwrap_err(); - assert!(err.contains("json parse"), "got: {err}"); + fn empty_rules_hold_every_call() { + let cfg = WorkerConfig { + rules: Vec::new(), + ..WorkerConfig::default() + }; + assert_eq!(cfg.permissions().rule_count(), 0); } #[test] - fn from_yaml_expands_env_var() { - std::env::set_var("APPROVAL_GATE_TEST_SWEEP", "*/5 * * * * *"); - let cfg = - WorkerConfig::from_yaml("sweep_expression: \"${APPROVAL_GATE_TEST_SWEEP}\"\n").unwrap(); - assert_eq!(cfg.sweep_expression, "*/5 * * * * *"); - std::env::remove_var("APPROVAL_GATE_TEST_SWEEP"); + fn default_rules_compile_into_a_matcher() { + let cfg = WorkerConfig::default(); + assert!(cfg.permissions().rule_count() > 0); } #[test] - fn boot_signature_equal_when_only_tuning_knobs_differ() { - let base = WorkerConfig::default(); - let tuned = WorkerConfig { - default_mode: PermissionMode::Auto, - pending_timeout_ms: base.pending_timeout_ms + 1, - policy_timeout_ms: base.policy_timeout_ms + 1, - ..base.clone() - }; - assert_eq!(base.boot_signature(), tuned.boot_signature()); + fn from_json_rejects_garbage() { + let err = WorkerConfig::from_json(&serde_json::json!({ "default_mode": 42 })).unwrap_err(); + assert!(err.contains("json parse"), "got: {err}"); + let err = WorkerConfig::from_json(&serde_json::json!("garbage")).unwrap_err(); + assert!(err.contains("json parse"), "got: {err}"); } #[test] - fn boot_signature_differs_on_structural_fields() { - let base = WorkerConfig::default(); - let rebound = WorkerConfig { - sweep_expression: "*/30 * * * * *".to_string(), - ..base.clone() - }; - let rehooked = WorkerConfig { - hook: HookBinding { - functions: vec!["shell::*".to_string()], - ..HookBinding::default() - }, - ..base.clone() - }; - assert_ne!(base.boot_signature(), rebound.boot_signature()); - assert_ne!(base.boot_signature(), rehooked.boot_signature()); + fn from_yaml_expands_env_var_in_rules() { + std::env::set_var("APPROVAL_GATE_TEST_RULE", "state::get"); + let cfg = WorkerConfig::from_yaml("rules:\n - \"${APPROVAL_GATE_TEST_RULE}\"\n").unwrap(); + assert_eq!(cfg.rules, vec![json!("state::get")]); + std::env::remove_var("APPROVAL_GATE_TEST_RULE"); } } diff --git a/approval-gate/src/configuration.rs b/approval-gate/src/configuration.rs index 9db6f5a35..c7e2b8998 100644 --- a/approval-gate/src/configuration.rs +++ b/approval-gate/src/configuration.rs @@ -3,19 +3,9 @@ //! changes. Mirrors [`context-manager`](../../context-manager/src/configuration.rs) / //! [`session-manager`](../../session-manager/src/configuration.rs). //! -//! Every field hot-reloads — nothing requires a restart: -//! -//! - `hook` and `sweep_expression` are the STRUCTURAL fields (the two -//! trigger bindings). On a change the config handler **re-binds** the -//! affected trigger live — it registers the new binding, then -//! `unregister()`s the old (a fail-safe overlap; the gate is idempotent, -//! so a brief double-fire is harmless) — keeping the [`Trigger`] handle -//! in [`TriggerHandles`]. -//! - Every OTHER field is a per-call tuning knob (the `*_timeout_ms` -//! budgets and the approval defaults `default_mode` / `always_allow_seed` -//! / `pending_timeout_ms`), read from the live snapshot per call via -//! [`Deps::config`](crate::functions::Deps::config); a change swaps the -//! snapshot. +//! Every configuration field hot-reloads via a snapshot swap — nothing +//! requires a restart. The harness `pre_trigger` hook binding is fixed at +//! worker startup (consult on all calls, fail closed). //! //! `configuration` is a REQUIRED boot dependency: a failed register/fetch //! aborts startup (the gate must run on a known, authoritative policy @@ -24,7 +14,7 @@ use std::sync::Arc; use std::time::Duration; -use iii_sdk::{IIIError, RegisterFunction, RegisterTriggerInput, Trigger, TriggerRequest, III}; +use iii_sdk::{IIIError, RegisterFunction, RegisterTriggerInput, TriggerRequest, III}; use serde_json::{json, Value}; use tokio::sync::RwLock; @@ -39,12 +29,17 @@ pub type ConfigCell = Arc>>; pub const CONFIG_ID: &str = "approval-gate"; const CONFIG_FN_ID: &str = "approval::on-config-change"; -const CONFIG_TIMEOUT_MS: u64 = 5_000; const CONFIG_RETRIES: u32 = 3; /// Base backoff between configuration RPC retries; multiplied by the /// attempt number for a linear backoff (250ms, 500ms, …). const CONFIG_RETRY_BACKOFF_MS: u64 = 250; +/// Fixed `harness::hook::pre-trigger` binding — approval-gate consults on +/// every function call. +const HOOK_FUNCTIONS: &[&str] = &["*"]; +const HOOK_TIMEOUT_MS: u64 = 5_000; +const HOOK_ON_ERROR: &str = "fail_closed"; + /// Register the `approval-gate` configuration schema with the /// configuration worker. When `seed` is present, its value is installed /// as `initial_value`. Otherwise, the built-in default is seeded only @@ -54,10 +49,10 @@ pub async fn register_config(iii: &III, seed: Option<&WorkerConfig>) -> Result<( let mut payload = json!({ "id": CONFIG_ID, "name": "Approval Gate", - "description": "Policy and decision surface settings: the harness hook binding and \ - expiry-sweep schedule, the RPC timeout budgets, and the deployment \ - approval defaults (permission mode for new sessions, the auto-mode \ - trust seed, and the pending-hold timeout).", + "description": "Policy and decision surface settings: the deployment \ + approval defaults (permission mode for new sessions \ + and the auto-mode trust seed) and the agent permission \ + rules.", "schema": WorkerConfig::json_schema(), }); if let Some(seed) = seed { @@ -105,82 +100,34 @@ async fn try_get_config_value(iii: &III) -> Result, String> { } } -/// Swap the config snapshot under the write lock. Per-call tuning knobs -/// take effect on the next read; any trigger re-bind is done by the caller -/// before this swap. +/// Swap the config snapshot under the write lock. pub async fn apply_config(cell: &ConfigCell, cfg: WorkerConfig) { *cell.write().await = Arc::new(cfg); } -/// Live handles for the two hot-reloadable trigger bindings — the harness -/// hook and the cron sweep. The config handler re-binds them on a `hook` / -/// `sweep_expression` change (register the new binding, then -/// `unregister()` the old), so neither needs a restart. -pub struct TriggerHandles { - pub hook: std::sync::Mutex>, - pub sweep: std::sync::Mutex>, -} - -/// Register a best-effort binding and return its handle. The trigger type -/// may not exist yet (standalone deployment); that surfaces later as an -/// async `trigger_type_not_found` log, never an `Err` here, so a `None` -/// means "no handle to retain" rather than a hard failure. -fn bind(iii: &III, trigger_type: &str, function_id: &str, config: Value) -> Option { +/// Bind the fixed `harness::hook::pre-trigger` hook at worker startup. +pub fn bind_hook(iii: &III) { match iii.register_trigger(RegisterTriggerInput { - trigger_type: trigger_type.to_string(), - function_id: function_id.to_string(), - config, + trigger_type: "harness::hook::pre-trigger".to_string(), + function_id: "approval::gate".to_string(), + config: json!({ + "functions": HOOK_FUNCTIONS, + "timeout_ms": HOOK_TIMEOUT_MS, + "on_error": HOOK_ON_ERROR, + }), metadata: None, }) { - Ok(handle) => { - tracing::info!(trigger_type, function_id, "trigger binding requested"); - Some(handle) - } - Err(e) => { - tracing::warn!(trigger_type, function_id, error = %e, "trigger binding failed (sibling absent?)"); - None - } - } -} - -/// (Re)bind the `harness::hook::pre-trigger` hook from the current config. -pub fn bind_hook(iii: &III, cfg: &WorkerConfig) -> Option { - bind( - iii, - "harness::hook::pre-trigger", - "approval::gate", - json!({ - "functions": cfg.hook.functions, - "timeout_ms": cfg.hook.timeout_ms, - "on_error": cfg.hook.on_error, - }), - ) -} - -/// (Re)bind the cron expiry sweep from the current config. -pub fn bind_sweep(iii: &III, cfg: &WorkerConfig) -> Option { - bind( - iii, - "cron", - "approval::sweep", - json!({ "expression": cfg.sweep_expression }), - ) -} - -/// Store the freshly-registered handle, then unregister the old one -/// (register-new-then-unregister-old: a fail-safe overlap — the gate is -/// never left unbound). A `None` new handle means the re-registration -/// didn't produce one; keep the old binding rather than tearing it down. -fn rebind_slot(slot: &std::sync::Mutex>, new: Option) { - let Some(new) = new else { - return; - }; - let old = slot - .lock() - .unwrap_or_else(|poison| poison.into_inner()) - .replace(new); - if let Some(old) = old { - old.unregister(); + Ok(_) => tracing::info!( + trigger_type = "harness::hook::pre-trigger", + function_id = "approval::gate", + "trigger binding requested" + ), + Err(e) => tracing::warn!( + trigger_type = "harness::hook::pre-trigger", + function_id = "approval::gate", + error = %e, + "trigger binding failed (sibling absent?)" + ), } } @@ -202,33 +149,24 @@ pub struct OnConfigChangeResponse { } /// Register the internal config-change handler and bind a `configuration` -/// trigger. `handles` holds the live hook + cron `Trigger`s the handler -/// re-binds when `hook` / `sweep_expression` change. The handler re-fetches -/// via `configuration::get` and ignores the trigger payload, so a direct -/// call can never inject config. -pub fn register_config_trigger( - iii: &III, - cell: ConfigCell, - handles: Arc, -) -> Result<(), IIIError> { +/// trigger. The handler re-fetches via `configuration::get` and ignores the +/// trigger payload, so a direct call can never inject config. +pub fn register_config_trigger(iii: &III, cell: ConfigCell) -> Result<(), IIIError> { let cell_for_fn = cell.clone(); - let handles_for_fn = handles.clone(); let engine = iii.clone(); iii.register_function( CONFIG_FN_ID, RegisterFunction::new_async(move |_event: OnConfigChangeEvent| { let cell = cell_for_fn.clone(); - let handles = handles_for_fn.clone(); let engine = engine.clone(); async move { - on_config_change(&engine, &cell, &handles).await; + on_config_change(&engine, &cell).await; Ok::(OnConfigChangeResponse { ok: true }) } }) .description( "Internal: hot-reload approval-gate from the authoritative configuration when it \ - changes — re-binds the harness hook / cron sweep on a hook or sweep_expression \ - change and swaps the per-call snapshot (timeouts + approval defaults) otherwise.", + changes — swaps the per-call snapshot (timeouts + approval defaults).", ), ); @@ -251,12 +189,7 @@ pub fn register_config_trigger( /// trusting `payload.new_value` would let any caller inject arbitrary /// config without updating persisted state. Re-fetch the stored value via /// `configuration::get` instead. -/// -/// `hook` / `sweep_expression` changes re-bind the affected trigger live -/// (register-new-then-unregister-old); every other field hot-applies via -/// the snapshot swap. The previous config is always kept on a fetch -/// failure. -async fn on_config_change(iii: &III, cell: &ConfigCell, handles: &TriggerHandles) { +async fn on_config_change(iii: &III, cell: &ConfigCell) { let cfg = match fetch_config(iii).await { Ok(cfg) => cfg, Err(e) => { @@ -268,20 +201,6 @@ async fn on_config_change(iii: &III, cell: &ConfigCell, handles: &TriggerHandles } }; - // Re-bind only the structural binding(s) that actually changed, so an - // unrelated tuning change never churns the security hook. - let old = cell.read().await.clone(); - if old.boot_signature() != cfg.boot_signature() { - if old.hook != cfg.hook { - rebind_slot(&handles.hook, bind_hook(iii, &cfg)); - tracing::info!("approval-gate hook re-bound (hook config changed)"); - } - if old.sweep_expression != cfg.sweep_expression { - rebind_slot(&handles.sweep, bind_sweep(iii, &cfg)); - tracing::info!("approval-gate sweep re-bound (sweep_expression changed)"); - } - } - apply_config(cell, cfg).await; tracing::info!("approval-gate configuration reloaded"); } @@ -294,7 +213,7 @@ async fn trigger_with_retry(iii: &III, function_id: &str, payload: Value) -> Res function_id: function_id.to_string(), payload: payload.clone(), action: None, - timeout_ms: Some(CONFIG_TIMEOUT_MS), + timeout_ms: None, }) .await { @@ -338,31 +257,4 @@ mod tests { apply_config(&cell, tuned).await; assert_eq!(cell.read().await.default_mode, PermissionMode::Full); } - - #[test] - fn rebind_slot_registers_new_then_unregisters_old() { - use std::sync::atomic::{AtomicUsize, Ordering}; - - // The old handle's unregister closure bumps a counter, proving - // register-new-then-unregister-old tears the old binding down only - // after the new one is stored. - let unregistered = Arc::new(AtomicUsize::new(0)); - let counter = unregistered.clone(); - let old = Trigger::new(Arc::new(move || { - counter.fetch_add(1, Ordering::SeqCst); - })); - let slot = std::sync::Mutex::new(Some(old)); - - rebind_slot(&slot, Some(Trigger::new(Arc::new(|| {})))); - assert_eq!( - unregistered.load(Ordering::SeqCst), - 1, - "the previous binding must be unregistered after the new one is stored" - ); - - // A `None` new handle (re-registration produced nothing) keeps the - // current binding untouched. - rebind_slot(&slot, None); - assert_eq!(unregistered.load(Ordering::SeqCst), 1); - } } diff --git a/approval-gate/src/decision.rs b/approval-gate/src/decision.rs index 0bec08b7c..78929e15c 100644 --- a/approval-gate/src/decision.rs +++ b/approval-gate/src/decision.rs @@ -8,7 +8,7 @@ //! 2. mode full → allow //! 3. approved_always hit → allow (every mode) //! 4. mode auto AND always_allow hit → allow (dormant under manual) -//! 5. fall through to the yaml policy (policy.rs) +//! 5. fall through to configuration `rules` (permissions module) use crate::types::{AlwaysAllowEntry, ApprovalSettings, PermissionMode}; @@ -17,7 +17,7 @@ use crate::types::{AlwaysAllowEntry, ApprovalSettings, PermissionMode}; /// defense). Prefix match deliberately broadens the prior art's /// six-function list. pub fn is_human_only(function_id: &str) -> bool { - function_id.starts_with("approval::") || function_id.starts_with("configuration::") + function_id.starts_with("approval::") } /// `*`-glob match with an equality fast-path. Globs exist because @@ -58,8 +58,8 @@ fn list_matches(entries: &[AlwaysAllowEntry], function_id: &str) -> bool { .any(|entry| glob_match(&entry.function_id, function_id)) } -/// Steps 2-4: the pre-policy short-circuits over one settings snapshot. -/// `false` = no short-circuit — fall through to the yaml policy. +/// Steps 2-4: the pre-rules short-circuits over one settings snapshot. +/// `false` = no short-circuit — fall through to configuration rules. pub fn pre_policy_allow(settings: &ApprovalSettings, function_id: &str) -> bool { if settings.mode == PermissionMode::Full { return true; @@ -102,10 +102,10 @@ mod tests { } #[test] - fn human_only_covers_approval_and_configuration_prefixes() { + fn human_only_covers_approval_prefix() { assert!(is_human_only("approval::set-mode")); assert!(is_human_only("approval::resolve")); - assert!(is_human_only("configuration::set")); + assert!(!is_human_only("configuration::set")); assert!(!is_human_only("shell::run")); assert!(!is_human_only("approvals::other")); } diff --git a/approval-gate/src/error.rs b/approval-gate/src/error.rs index 2bdfc55d6..ca6be2160 100644 --- a/approval-gate/src/error.rs +++ b/approval-gate/src/error.rs @@ -16,7 +16,7 @@ pub enum ApprovalError { StateUnavailable(String), /// `harness::function::resolve` failed; the pending record is kept - /// so the decision stays resolvable (or sweepable). + /// so the decision stays resolvable until turn/session cleanup. #[error("approval/harness_unavailable: {0}")] HarnessUnavailable(String), } diff --git a/approval-gate/src/events.rs b/approval-gate/src/events.rs index 44a4df452..03e471126 100644 --- a/approval-gate/src/events.rs +++ b/approval-gate/src/events.rs @@ -386,9 +386,6 @@ mod tests { "function_id": "shell::run", "arguments_excerpt": {}, "pending_at": 100, - // Live record so the resolve path (not the sweep) - // owns it and emits the resolved event under test. - "expires_at": crate::types::now_ms() + 1_800_000, "depth": 0, }), ) diff --git a/approval-gate/src/functions/add_always_allow.rs b/approval-gate/src/functions/add_always_allow.rs index 12114e10d..cde8cb70b 100644 --- a/approval-gate/src/functions/add_always_allow.rs +++ b/approval-gate/src/functions/add_always_allow.rs @@ -16,17 +16,14 @@ pub async fn handle( )); } let cfg = deps.config().await; - let settings = settings::materialize_and( - deps.iii.as_ref(), - &req.session_id, - &cfg, - cfg.state_timeout_ms, - |base, now| ApprovalSettings { - always_allow: with_grant(&base.always_allow, &req.function_id, now), - ..base - }, - ) - .await?; + let settings = + settings::materialize_and(deps.iii.as_ref(), &req.session_id, &cfg, |base, now| { + ApprovalSettings { + always_allow: with_grant(&base.always_allow, &req.function_id, now), + ..base + } + }) + .await?; Ok(SettingsResponse { settings }) } diff --git a/approval-gate/src/functions/approve_always.rs b/approval-gate/src/functions/approve_always.rs index eaf0f7606..8002eb58a 100644 --- a/approval-gate/src/functions/approve_always.rs +++ b/approval-gate/src/functions/approve_always.rs @@ -18,17 +18,14 @@ pub async fn handle( )); } let cfg = deps.config().await; - let settings = settings::materialize_and( - deps.iii.as_ref(), - &req.session_id, - &cfg, - cfg.state_timeout_ms, - |base, now| ApprovalSettings { - approved_always: with_grant(&base.approved_always, &req.function_id, now), - ..base - }, - ) - .await?; + let settings = + settings::materialize_and(deps.iii.as_ref(), &req.session_id, &cfg, |base, now| { + ApprovalSettings { + approved_always: with_grant(&base.approved_always, &req.function_id, now), + ..base + } + }) + .await?; Ok(SettingsResponse { settings }) } diff --git a/approval-gate/src/functions/clear_settings.rs b/approval-gate/src/functions/clear_settings.rs index b7fee3c8c..598d6e756 100644 --- a/approval-gate/src/functions/clear_settings.rs +++ b/approval-gate/src/functions/clear_settings.rs @@ -11,8 +11,7 @@ pub async fn handle( deps: &Deps, req: ClearSettingsRequest, ) -> Result { - let cfg = deps.config().await; - let cleared = settings::clear(deps.iii.as_ref(), &req.session_id, cfg.state_timeout_ms).await?; + let cleared = settings::clear(deps.iii.as_ref(), &req.session_id).await?; Ok(ClearSettingsResponse { cleared }) } diff --git a/approval-gate/src/functions/gate.rs b/approval-gate/src/functions/gate.rs index 4fb55accc..ced80f8d9 100644 --- a/approval-gate/src/functions/gate.rs +++ b/approval-gate/src/functions/gate.rs @@ -10,12 +10,11 @@ //! no-op on the existing record (and emits no second `pending_created`). use super::Deps; -use crate::config::WorkerConfig; use crate::decision; use crate::denial::{gate_unavailable_envelope, human_only_denial, permissions_deny_envelope}; use crate::error::ApprovalError; use crate::pending; -use crate::policy::{self, PolicyOutcome}; +use crate::permissions::Decision; use crate::redact::redact; use crate::session; use crate::settings; @@ -31,7 +30,7 @@ pub async fn handle(deps: &Deps, input: HookInput) -> Result Result Result Ok(HookOutput::Continue), - PolicyOutcome::Deny { + Decision::Allow { .. } => Ok(HookOutput::Continue), + Decision::Deny { rule_id, matched_constraint, } => { @@ -83,11 +78,7 @@ pub async fn handle(deps: &Deps, input: HookInput) -> Result { - let envelope = gate_unavailable_envelope(&call.function_id, &why); - Ok(deny(&envelope.reason)) - } - PolicyOutcome::NeedsApproval => Ok(hold(deps, &input, &call, &cfg).await), + Decision::NeedsApproval => Ok(hold(deps, &input, &call).await), } } @@ -102,15 +93,17 @@ fn deny(reason: &str) -> HookOutput { /// inbox. Write failure → fail-closed deny, never hold blind. /// `pending_created` emits asynchronously after the record is written — /// notification fan-out never blocks the trigger hot path. -async fn hold(deps: &Deps, input: &HookInput, call: &HookCall, cfg: &WorkerConfig) -> HookOutput { +/// +/// Holds never expire: the hook returns `{ decision: "hold" }` with no +/// timeout. A held call stays held until a human resolves it or turn/session +/// cleanup collects it. +async fn hold(deps: &Deps, input: &HookInput, call: &HookCall) -> HookOutput { let iii = deps.iii.as_ref(); // Idempotency: a redelivered step re-holds the same call. - match pending::get(iii, &input.session_id, &call.id, cfg.state_timeout_ms).await { - Ok(Some(existing)) => { - return HookOutput::Hold { - pending_timeout_ms: (existing.expires_at - existing.pending_at).max(0), - }; + match pending::get(iii, &input.session_id, &call.id).await { + Ok(Some(_existing)) => { + return HookOutput::Hold; } Ok(None) => {} Err(e) => { @@ -123,7 +116,7 @@ async fn hold(deps: &Deps, input: &HookInput, call: &HookCall, cfg: &WorkerConfi } let (session_title, session_description, session_metadata) = - fetch_session_context(deps, &input.session_id, cfg.session_fetch_timeout_ms).await; + fetch_session_context(deps, &input.session_id).await; let pending_at = now_ms(); let record = PendingApprovalRecord { @@ -133,7 +126,6 @@ async fn hold(deps: &Deps, input: &HookInput, call: &HookCall, cfg: &WorkerConfi function_id: call.function_id.clone(), arguments_excerpt: redact(&call.arguments), pending_at, - expires_at: pending_at + cfg.pending_timeout_ms, session_title, session_description, session_metadata, @@ -141,7 +133,7 @@ async fn hold(deps: &Deps, input: &HookInput, call: &HookCall, cfg: &WorkerConfi assistant_excerpt: None, }; - match pending::put(iii, &record, cfg.state_timeout_ms).await { + match pending::put(iii, &record).await { Err(e) => { let envelope = gate_unavailable_envelope( &call.function_id, @@ -151,35 +143,24 @@ async fn hold(deps: &Deps, input: &HookInput, call: &HookCall, cfg: &WorkerConfi } // Lost a write race with a concurrent duplicate hold: the first // writer's record (and emission) stands. - Ok(Some(_prior)) => HookOutput::Hold { - pending_timeout_ms: cfg.pending_timeout_ms, - }, + Ok(Some(_prior)) => HookOutput::Hold, Ok(None) => { let sink = deps.sink.clone(); tokio::spawn(async move { sink.pending_created(&record).await; }); - HookOutput::Hold { - pending_timeout_ms: cfg.pending_timeout_ms, - } + HookOutput::Hold } } } /// Best-effort `session::get` — fields are omitted on any failure -/// (session-manager absent, timeout, unknown session), within its own -/// budget so it can't eat the hook's. +/// (session-manager absent, timeout, unknown session). async fn fetch_session_context( deps: &Deps, session_id: &str, - session_fetch_timeout_ms: u64, ) -> (Option, Option, Option) { - let reply = session::get( - deps.iii.as_ref(), - session_id, - Some(session_fetch_timeout_ms), - ) - .await; + let reply = session::get(deps.iii.as_ref(), session_id).await; let Ok(reply) = reply else { return (None, None, None); }; @@ -231,7 +212,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread")] - async fn full_mode_allows_without_consulting_policy() { + async fn full_mode_allows_without_consulting_rules() { with_stack(BootOpts::needs_approval(), |stack| async move { seed_settings( &stack.iii, @@ -264,7 +245,7 @@ mod tests { ) .await; let out = handle(&stack.deps, hook_input("shell::run")).await.unwrap(); - assert!(matches!(out, HookOutput::Hold { .. })); + assert!(matches!(out, HookOutput::Hold)); let record = state_get(&stack.iii, PENDING_SCOPE, "s_1/c_1").await; assert_eq!(record["function_id"], json!("shell::run")); }) @@ -296,39 +277,42 @@ mod tests { } #[tokio::test(flavor = "multi_thread")] - async fn policy_allow_continues() { - with_stack( - BootOpts::policy_reply(json!({ "decision": "allow", "rule_id": "r1" })), - |stack| async move { - assert_eq!( - handle(&stack.deps, hook_input("shell::run")).await.unwrap(), - HookOutput::Continue - ); - }, - ) + async fn config_rules_allow_continues() { + with_stack(BootOpts::allow(), |stack| async move { + assert_eq!( + handle(&stack.deps, hook_input("shell::run")).await.unwrap(), + HookOutput::Continue + ); + }) .await; } #[tokio::test(flavor = "multi_thread")] - async fn policy_transport_failure_fails_closed() { - with_stack( - BootOpts::policy_error("connection refused"), - |stack| async move { - assert!(matches!( - handle(&stack.deps, hook_input("shell::run")).await.unwrap(), - HookOutput::Deny { .. } - )); - }, - ) + async fn config_rules_deny_denies() { + with_stack(BootOpts::deny_function("shell::run"), |stack| async move { + assert!(matches!( + handle(&stack.deps, hook_input("shell::run")).await.unwrap(), + HookOutput::Deny { .. } + )); + }) .await; } #[tokio::test(flavor = "multi_thread")] - async fn missing_policy_worker_fails_closed() { - with_stack(BootOpts::no_policy(), |stack| async move { + async fn empty_rules_hold_under_manual_mode() { + with_stack(BootOpts::needs_approval(), |stack| async move { + seed_settings( + &stack.iii, + "s_1", + &ApprovalSettings { + mode: PermissionMode::Manual, + ..Default::default() + }, + ) + .await; assert!(matches!( handle(&stack.deps, hook_input("shell::run")).await.unwrap(), - HookOutput::Deny { .. } + HookOutput::Hold )); }) .await; @@ -348,12 +332,12 @@ mod tests { .await; assert!(matches!( handle(&stack.deps, hook_input("shell::run")).await.unwrap(), - HookOutput::Hold { .. } + HookOutput::Hold )); let before = state_get(&stack.iii, PENDING_SCOPE, "s_1/c_1").await; assert!(matches!( handle(&stack.deps, hook_input("shell::run")).await.unwrap(), - HookOutput::Hold { .. } + HookOutput::Hold )); assert_eq!( state_get(&stack.iii, PENDING_SCOPE, "s_1/c_1").await, diff --git a/approval-gate/src/functions/get_pending.rs b/approval-gate/src/functions/get_pending.rs index fe4a3e53a..814cd3e24 100644 --- a/approval-gate/src/functions/get_pending.rs +++ b/approval-gate/src/functions/get_pending.rs @@ -12,15 +12,9 @@ pub async fn handle( ) -> Result, ApprovalError> { validate_id("session_id", &req.session_id)?; validate_id("function_call_id", &req.function_call_id)?; - let cfg = deps.config().await; - let record = pending::get( - deps.iii.as_ref(), - &req.session_id, - &req.function_call_id, - cfg.state_timeout_ms, - ) - .await - .map_err(|e| ApprovalError::StateUnavailable(format!("pending record read failed: {e}")))?; + let record = pending::get(deps.iii.as_ref(), &req.session_id, &req.function_call_id) + .await + .map_err(|e| ApprovalError::StateUnavailable(format!("pending record read failed: {e}")))?; Ok(record.map(|pending| GetPendingResponse { pending })) } @@ -56,7 +50,6 @@ mod tests { "function_call_id": "c_1", "function_id": "shell::run", "pending_at": 1, - "expires_at": 2, }), ) .await; diff --git a/approval-gate/src/functions/get_settings.rs b/approval-gate/src/functions/get_settings.rs index 0706ebfd7..5e7419f9d 100644 --- a/approval-gate/src/functions/get_settings.rs +++ b/approval-gate/src/functions/get_settings.rs @@ -12,8 +12,7 @@ pub async fn handle( ) -> Result { validate_id("session_id", &req.session_id)?; let cfg = deps.config().await; - let stored = - settings::read_strict(deps.iii.as_ref(), &req.session_id, cfg.state_timeout_ms).await?; + let stored = settings::read_strict(deps.iii.as_ref(), &req.session_id).await?; let (settings, source) = settings::effective(stored, &cfg); Ok(GetSettingsResponse { settings, source }) } diff --git a/approval-gate/src/functions/list_pending.rs b/approval-gate/src/functions/list_pending.rs index 99e874ac6..bf6b42747 100644 --- a/approval-gate/src/functions/list_pending.rs +++ b/approval-gate/src/functions/list_pending.rs @@ -54,8 +54,7 @@ pub async fn handle( let after = req.cursor.as_deref().map(decode_cursor).transpose()?; let limit = req.limit.unwrap_or(DEFAULT_LIMIT).clamp(1, MAX_LIMIT); - let cfg = deps.config().await; - let mut records = pending::list_all(deps.iii.as_ref(), cfg.state_timeout_ms) + let mut records = pending::list_all(deps.iii.as_ref()) .await .map_err(|e| ApprovalError::StateUnavailable(format!("pending list failed: {e}")))?; @@ -112,7 +111,6 @@ mod tests { "function_id": "shell::run", "arguments_excerpt": {}, "pending_at": pending_at, - "expires_at": pending_at + 1000, "depth": 0, }); if let Some(owner) = owner { diff --git a/approval-gate/src/functions/mod.rs b/approval-gate/src/functions/mod.rs index ba34c58df..646342c10 100644 --- a/approval-gate/src/functions/mod.rs +++ b/approval-gate/src/functions/mod.rs @@ -14,7 +14,6 @@ pub mod on_turn_completed; pub mod purge; pub mod resolve; pub mod set_mode; -pub mod sweep; pub mod add_always_allow; pub mod remove_always_allow; @@ -79,9 +78,6 @@ pub const ON_TURN_COMPLETED_ID: &str = "approval::on-turn-completed"; pub const ON_TURN_COMPLETED_DESC: &str = "Internal: harness::turn-completed handler (purge the turn's pending records)."; -pub const SWEEP_ID: &str = "approval::sweep"; -pub const SWEEP_DESC: &str = "Internal: cron handler (expire pending records past expires_at)."; - /// Everything a function handler needs. pub struct Deps { pub iii: Arc, @@ -200,9 +196,6 @@ pub fn register_all(iii: &Arc, deps: &Arc) { ON_TURN_COMPLETED_DESC, |d, r| async move { on_turn_completed::handle(&d, r).await }, ); - register(iii, deps, SWEEP_ID, SWEEP_DESC, |d, r| async move { - sweep::handle(&d, r).await - }); tracing::info!("all approval::* functions registered"); } @@ -275,7 +268,6 @@ pub fn catalog() -> Vec { spec::(CLEAR_SETTINGS_ID, CLEAR_SETTINGS_DESC), spec::(ON_SESSION_DELETED_ID, ON_SESSION_DELETED_DESC), spec::(ON_TURN_COMPLETED_ID, ON_TURN_COMPLETED_DESC), - spec::(SWEEP_ID, SWEEP_DESC), ] } diff --git a/approval-gate/src/functions/on_session_deleted.rs b/approval-gate/src/functions/on_session_deleted.rs index 30e28455e..7f22af29f 100644 --- a/approval-gate/src/functions/on_session_deleted.rs +++ b/approval-gate/src/functions/on_session_deleted.rs @@ -19,10 +19,7 @@ pub async fn handle( deps: &Deps, event: SessionDeletedEvent, ) -> Result { - let cfg = deps.config().await; - if let Err(e) = - settings::clear(deps.iii.as_ref(), &event.session_id, cfg.state_timeout_ms).await - { + if let Err(e) = settings::clear(deps.iii.as_ref(), &event.session_id).await { tracing::warn!(session_id = %event.session_id, error = %e, "settings purge failed"); } let purged = purge::purge_matching(deps, |r| r.session_id == event.session_id).await; @@ -50,7 +47,6 @@ mod tests { "function_call_id": cid, "function_id": "shell::run", "pending_at": 1, - "expires_at": 2, }), ) .await; diff --git a/approval-gate/src/functions/on_turn_completed.rs b/approval-gate/src/functions/on_turn_completed.rs index 216dde1d3..26ea1ab43 100644 --- a/approval-gate/src/functions/on_turn_completed.rs +++ b/approval-gate/src/functions/on_turn_completed.rs @@ -49,7 +49,6 @@ mod tests { "function_call_id": cid, "function_id": "shell::run", "pending_at": 1, - "expires_at": 2, }), ) .await; diff --git a/approval-gate/src/functions/purge.rs b/approval-gate/src/functions/purge.rs index bc740c6d1..d2918f430 100644 --- a/approval-gate/src/functions/purge.rs +++ b/approval-gate/src/functions/purge.rs @@ -11,26 +11,18 @@ pub async fn purge_matching(deps: &Deps, predicate: F) -> usize where F: Fn(&PendingApprovalRecord) -> bool, { - let cfg = deps.config().await; let iii = deps.iii.as_ref(); - let records = match pending::list_all(iii, cfg.state_timeout_ms).await { + let records = match pending::list_all(iii).await { Ok(records) => records, Err(e) => { - tracing::warn!(error = %e, "purge: pending list failed; sweep will retry"); + tracing::warn!(error = %e, "purge: pending list failed; retry on next event"); return 0; } }; let mut purged = 0usize; for record in records.into_iter().filter(|r| predicate(r)) { - match pending::delete_with_gate( - iii, - &record.session_id, - &record.function_call_id, - cfg.state_timeout_ms, - ) - .await - { + match pending::delete_with_gate(iii, &record.session_id, &record.function_call_id).await { Ok(Some(deleted)) => { purged += 1; deps.sink @@ -53,7 +45,7 @@ where session_id = %record.session_id, function_call_id = %record.function_call_id, error = %e, - "purge: delete failed; sweep will collect the record" + "purge: delete failed; retry or turn/session cleanup will collect the record" ); } } diff --git a/approval-gate/src/functions/remove_always_allow.rs b/approval-gate/src/functions/remove_always_allow.rs index 1587101e2..8d4c20adf 100644 --- a/approval-gate/src/functions/remove_always_allow.rs +++ b/approval-gate/src/functions/remove_always_allow.rs @@ -13,17 +13,14 @@ pub async fn handle( req: AlwaysAllowMutationRequest, ) -> Result { let cfg = deps.config().await; - let settings = settings::materialize_and( - deps.iii.as_ref(), - &req.session_id, - &cfg, - cfg.state_timeout_ms, - |base, _now| ApprovalSettings { - always_allow: without_grant(&base.always_allow, &req.function_id), - ..base - }, - ) - .await?; + let settings = + settings::materialize_and(deps.iii.as_ref(), &req.session_id, &cfg, |base, _now| { + ApprovalSettings { + always_allow: without_grant(&base.always_allow, &req.function_id), + ..base + } + }) + .await?; Ok(SettingsResponse { settings }) } @@ -41,8 +38,10 @@ mod tests { with_stack(BootOpts::needs_approval(), |stack| async move { *stack.config.write().await = Arc::new(WorkerConfig { default_mode: PermissionMode::Auto, - always_allow_seed: vec!["state::get".into(), "shell::run".into()], - ..WorkerConfig::default() + rules: vec![ + serde_json::json!({"function": "state::get", "action": "allow", "modes": ["auto"]}), + serde_json::json!({"function": "shell::run", "action": "allow", "modes": ["auto"]}), + ], }); let res = handle( &stack.deps, diff --git a/approval-gate/src/functions/resolve.rs b/approval-gate/src/functions/resolve.rs index 6b187f105..22823fe54 100644 --- a/approval-gate/src/functions/resolve.rs +++ b/approval-gate/src/functions/resolve.rs @@ -5,8 +5,8 @@ //! with the resolution. //! //! Crash ordering: `harness::function::resolve` FIRST, then delete, then -//! emit — a crash between the first two leaks one record until the sweep -//! collects it; it can never lose a decision. +//! emit — a crash between the first two leaks one record until turn/session +//! cleanup; it can never lose a decision. use serde_json::json; @@ -24,16 +24,10 @@ pub async fn handle(deps: &Deps, req: ResolveRequest) -> Result Result json!({ "session_id": req.session_id, @@ -82,25 +62,16 @@ pub async fn handle(deps: &Deps, req: ResolveRequest) -> Result { deps.sink .pending_resolved(&PendingResolvedEvent { @@ -125,13 +96,13 @@ pub async fn handle(deps: &Deps, req: ResolveRequest) -> Result {} Err(e) => { - // The decision reached the harness; the orphaned record is - // sweep food. Never fail the resolve over cleanup. + // The decision reached the harness; retry or turn/session cleanup + // will collect the orphaned record. Never fail resolve over cleanup. tracing::warn!( session_id = %req.session_id, function_call_id = %req.function_call_id, error = %e, - "pending record delete failed after resolve; sweep will collect it" + "pending record delete failed after resolve; retry or purge will collect it" ); } } @@ -159,9 +130,6 @@ mod tests { function_id: "shell::run".into(), arguments_excerpt: json!({ "cmd": "ls" }), pending_at: 100, - // Live record: deadline well in the future so the resolve path - // (not the sweep) owns it. - expires_at: now_ms() + 1_800_000, session_title: None, session_description: None, session_metadata: Some(serde_json::from_value(json!({ "owner": "u_1" })).unwrap()), @@ -234,49 +202,6 @@ mod tests { .await; } - #[tokio::test(flavor = "multi_thread")] - async fn expired_record_is_left_for_the_sweep() { - with_stack(BootOpts::needs_approval(), |stack| async move { - // A record whose deadline has already passed belongs to the - // sweep, not to a late human resolve. - let record = PendingApprovalRecord { - session_id: "s_1".into(), - turn_id: "t_9".into(), - function_call_id: "c_1".into(), - function_id: "shell::run".into(), - arguments_excerpt: json!({ "cmd": "ls" }), - pending_at: 100, - expires_at: 200, - session_title: None, - session_description: None, - session_metadata: None, - depth: 0, - assistant_excerpt: None, - }; - state_set( - &stack.iii, - PENDING_SCOPE, - "s_1/c_1", - serde_json::to_value(record).unwrap(), - ) - .await; - - let res = handle(&stack.deps, req(ResolveDecision::Allow, None)) - .await - .unwrap(); - assert!(!res.resolved, "expired record must not resolve here"); - assert!( - log_snapshot(&stack.harness_calls).is_empty(), - "no harness call: the sweep owns expired records" - ); - // Left intact for the sweep to collect. - assert!(!state_get(&stack.iii, PENDING_SCOPE, "s_1/c_1") - .await - .is_null()); - }) - .await; - } - #[tokio::test(flavor = "multi_thread")] async fn invalid_ids_are_rejected() { with_stack(BootOpts::needs_approval(), |stack| async move { diff --git a/approval-gate/src/functions/set_mode.rs b/approval-gate/src/functions/set_mode.rs index d617569ab..04adea8ba 100644 --- a/approval-gate/src/functions/set_mode.rs +++ b/approval-gate/src/functions/set_mode.rs @@ -9,18 +9,15 @@ use crate::types::{ApprovalSettings, SetModeRequest, SettingsResponse}; pub async fn handle(deps: &Deps, req: SetModeRequest) -> Result { let cfg = deps.config().await; - let settings = settings::materialize_and( - deps.iii.as_ref(), - &req.session_id, - &cfg, - cfg.state_timeout_ms, - |base, now| ApprovalSettings { - mode: req.mode, - mode_set_at: now, - ..base - }, - ) - .await?; + let settings = + settings::materialize_and(deps.iii.as_ref(), &req.session_id, &cfg, |base, now| { + ApprovalSettings { + mode: req.mode, + mode_set_at: now, + ..base + } + }) + .await?; Ok(SettingsResponse { settings }) } diff --git a/approval-gate/src/functions/sweep.rs b/approval-gate/src/functions/sweep.rs deleted file mode 100644 index 2a9796cda..000000000 --- a/approval-gate/src/functions/sweep.rs +++ /dev/null @@ -1,202 +0,0 @@ -//! `approval::sweep` — the cron-bound GC backstop (approval-gate.md § -//! Sweep). For every record past `expires_at`: settle the held call with -//! a timeout denial, delete through the emit gate, fire -//! `pending_resolved { outcome: "timeout" }`. Also collects records -//! orphaned by a crash between resolve and delete — which is why no -//! delete path needs to be transactional. - -use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; -use serde_json::{json, Value}; - -use super::Deps; -use crate::error::ApprovalError; -use crate::harness; -use crate::pending; -use crate::types::{ - now_ms, text_block, PendingApprovalRecord, PendingResolvedEvent, ResolvedOutcome, -}; - -/// Input of `approval::sweep` — the cron tick carries no arguments. A struct -/// (not `Value`) keeps the request schema concrete; unknown fields (the cron -/// trigger payload, engine-injected ids) are ignored. -#[derive(Debug, Clone, Default, Deserialize, JsonSchema)] -pub struct SweepRequest {} - -/// Output of `approval::sweep`: how many expired records were collected. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] -pub struct SweepResponse { - pub swept: usize, -} - -pub async fn handle(deps: &Deps, _req: SweepRequest) -> Result { - let cfg = deps.config().await; - let iii = deps.iii.as_ref(); - let now = now_ms(); - - let records = match pending::list_all(iii, cfg.state_timeout_ms).await { - Ok(records) => records, - Err(e) => { - tracing::warn!(error = %e, "sweep: pending list failed; retrying next tick"); - return Ok(SweepResponse { swept: 0 }); - } - }; - - let mut swept = 0usize; - for record in records.into_iter().filter(|r| r.expires_at <= now) { - // Settle the call first — a no-op `{ resolved: false }` when the - // harness's own pending sweep or another path got there already. - // A transport failure is tolerated: the record is still deleted - // (the GC contract — the inbox must stay O(live)), and the - // harness sweep remains the backstop for the parked turn. - let resolve_payload = timeout_resolve_payload(&record); - if let Err(e) = - harness::function_resolve(iii, resolve_payload, Some(cfg.harness_timeout_ms)).await - { - tracing::warn!( - session_id = %record.session_id, - function_call_id = %record.function_call_id, - error = %e, - "sweep: harness resolve failed; deleting the expired record anyway" - ); - } - - match pending::delete_with_gate( - iii, - &record.session_id, - &record.function_call_id, - cfg.state_timeout_ms, - ) - .await - { - Ok(Some(deleted)) => { - swept += 1; - deps.sink - .pending_resolved(&PendingResolvedEvent { - session_id: deleted.session_id, - turn_id: deleted.turn_id, - function_call_id: deleted.function_call_id, - function_id: deleted.function_id, - outcome: ResolvedOutcome::Timeout, - reason: None, - session_metadata: deleted.session_metadata, - resolved_at: now_ms(), - }) - .await; - } - Ok(None) => {} - Err(e) => { - tracing::warn!( - session_id = %record.session_id, - function_call_id = %record.function_call_id, - error = %e, - "sweep: delete failed; retrying next tick" - ); - } - } - } - - if swept > 0 { - tracing::info!(swept, "sweep: expired pending approvals collected"); - } - Ok(SweepResponse { swept }) -} - -/// Not a `DenialEnvelope` — nobody denied the call; no human decision -/// arrived before the deadline. The text and details still give the -/// model something to adapt to. -fn timeout_resolve_payload(record: &PendingApprovalRecord) -> Value { - let window_ms = record.expires_at - record.pending_at; - let text = format!( - "Approval request for {} timed out: no human decision within {}ms. The call was not executed.", - record.function_id, window_ms - ); - json!({ - "session_id": record.session_id, - "turn_id": record.turn_id, - "function_call_id": record.function_call_id, - "action": "deliver", - "is_error": true, - "content": [text_block(text)], - "details": { - "status": "timeout", - "function_id": record.function_id, - "pending_at": record.pending_at, - "expires_at": record.expires_at, - }, - }) -} - -#[cfg(test)] -mod tests { - use serde_json::json; - - use super::*; - use crate::pending::PENDING_SCOPE; - use crate::testkit::{log_snapshot, state_get, state_set, with_stack, BootOpts}; - use crate::types::now_ms; - - #[tokio::test(flavor = "multi_thread")] - async fn sweeps_only_expired_records() { - with_stack(BootOpts::needs_approval(), |stack| async move { - state_set( - &stack.iii, - PENDING_SCOPE, - "s_1/c_expired", - json!({ - "session_id": "s_1", - "turn_id": "t_1", - "function_call_id": "c_expired", - "function_id": "shell::run", - "pending_at": 100, - "expires_at": 200, - "depth": 0, - }), - ) - .await; - state_set( - &stack.iii, - PENDING_SCOPE, - "s_1/c_live", - json!({ - "session_id": "s_1", - "turn_id": "t_1", - "function_call_id": "c_live", - "function_id": "shell::run", - "pending_at": 100, - "expires_at": now_ms() + 60_000, - "depth": 0, - }), - ) - .await; - - let res = handle(&stack.deps, SweepRequest::default()).await.unwrap(); - assert_eq!(res.swept, 1); - assert!(state_get(&stack.iii, PENDING_SCOPE, "s_1/c_expired") - .await - .is_null()); - assert!(!state_get(&stack.iii, PENDING_SCOPE, "s_1/c_live") - .await - .is_null()); - - let harness = log_snapshot(&stack.harness_calls); - assert_eq!(harness.len(), 1); - assert_eq!(harness[0]["details"]["status"], json!("timeout")); - assert!(wait_for_resolved(&stack).await, "expected pending_resolved"); - }) - .await; - } - - #[tokio::test(flavor = "multi_thread")] - async fn empty_scope_is_a_noop() { - with_stack(BootOpts::needs_approval(), |stack| async move { - let res = handle(&stack.deps, SweepRequest::default()).await.unwrap(); - assert_eq!(res.swept, 0); - }) - .await; - } - - async fn wait_for_resolved(stack: &crate::testkit::TestStack) -> bool { - crate::testkit::wait_for(3_000, || !log_snapshot(&stack.resolved).is_empty()).await - } -} diff --git a/approval-gate/src/harness.rs b/approval-gate/src/harness.rs index 0978dccaa..ba796fb74 100644 --- a/approval-gate/src/harness.rs +++ b/approval-gate/src/harness.rs @@ -3,16 +3,12 @@ use iii_sdk::{IIIError, TriggerRequest, III}; use serde_json::Value; -pub async fn function_resolve( - iii: &III, - payload: Value, - timeout_ms: Option, -) -> Result { +pub async fn function_resolve(iii: &III, payload: Value) -> Result { iii.trigger(TriggerRequest { function_id: "harness::function::resolve".into(), payload, action: None, - timeout_ms, + timeout_ms: None, }) .await } diff --git a/approval-gate/src/lib.rs b/approval-gate/src/lib.rs index 8bb628b46..bee0ff022 100644 --- a/approval-gate/src/lib.rs +++ b/approval-gate/src/lib.rs @@ -26,7 +26,7 @@ pub mod functions; pub mod harness; pub mod manifest; pub mod pending; -pub mod policy; +pub mod permissions; pub mod redact; pub mod session; pub mod settings; diff --git a/approval-gate/src/main.rs b/approval-gate/src/main.rs index 2548d8f73..45ac83a56 100644 --- a/approval-gate/src/main.rs +++ b/approval-gate/src/main.rs @@ -13,14 +13,13 @@ //! 4. Register the two custom trigger types (`approval::pending-created`, //! `approval::pending-resolved`) — first, because the function //! handlers capture the subscriber sets they fan out to. -//! 5. Register the 13 `approval::*` functions (each reads the live config +//! 5. Register the 12 `approval::*` functions (each reads the live config //! snapshot per call). -//! 6. Bind the gate hook + the session/turn/cron triggers, all +//! 6. Bind the fixed gate hook + the session/turn triggers, all //! best-effort (in a standalone deployment some of these trigger types -//! don't exist yet; the worker still boots and serves its RPCs). The -//! hook globs and sweep schedule come from the fetched config. +//! don't exist yet; the worker still boots and serves its RPCs). //! 7. Bind the `configuration` change trigger LAST so its handler closes -//! over the fully-built snapshot cell + boot signature. +//! over the fully-built snapshot cell. //! 8. Sleep on Ctrl+C, then `shutdown_async` cleanly. use std::sync::Arc; @@ -31,7 +30,7 @@ use iii_sdk::{register_worker, InitOptions, RegisterTriggerInput, WorkerMetadata use serde_json::json; use tokio::sync::RwLock; -use approval_gate::configuration::{self, ConfigCell, TriggerHandles}; +use approval_gate::configuration::{self, ConfigCell}; use approval_gate::events::{self, Emitter}; use approval_gate::functions::{self, Deps}; use approval_gate::{config, manifest}; @@ -164,15 +163,7 @@ async fn main() -> Result<()> { functions::register_all(&iii, &deps); - // The gate's own hook binding + the cron sweep are the two STRUCTURAL - // bindings — installing the worker is installing the hook - // (approval-gate.md § The approval::gate hook). Retain their Trigger - // handles so a `hook` / `sweep_expression` change re-binds them live - // (no restart; see configuration::register_config_trigger). - let handles = Arc::new(TriggerHandles { - hook: std::sync::Mutex::new(configuration::bind_hook(&iii, &cfg)), - sweep: std::sync::Mutex::new(configuration::bind_sweep(&iii, &cfg)), - }); + configuration::bind_hook(&iii); // These two carry no config and are never re-bound — best-effort only. bind_best_effort( @@ -189,12 +180,11 @@ async fn main() -> Result<()> { ); // LAST: bind the configuration-change trigger so its handler closes over - // the snapshot cell + the trigger handles it re-binds on a structural - // change. - configuration::register_config_trigger(&iii, cell, handles) + // the snapshot cell. + configuration::register_config_trigger(&iii, cell) .context("registering the configuration change trigger")?; - tracing::info!("approval-gate ready: 13 approval::* functions + 2 custom trigger types"); + tracing::info!("approval-gate ready: 12 approval::* functions + 2 custom trigger types"); tokio::signal::ctrl_c().await?; tracing::info!("approval-gate shutting down"); diff --git a/approval-gate/src/manifest.rs b/approval-gate/src/manifest.rs index 1016faa68..2eaed9010 100644 --- a/approval-gate/src/manifest.rs +++ b/approval-gate/src/manifest.rs @@ -18,22 +18,9 @@ pub fn build_manifest() -> ModuleManifest { description: "Policy and decision surface for human-held function calls — pre_trigger gate, pending inbox, per-session permission settings, and two notification trigger types." .to_string(), - // Mirrors config::WorkerConfig::default() field-for-field. - default_config: serde_json::json!({ - "hook": { - "functions": ["*"], - "timeout_ms": 5000, - "on_error": "fail_closed", - }, - "sweep_expression": "0 0 0 * * *", - "policy_timeout_ms": 5000, - "session_fetch_timeout_ms": 1000, - "state_timeout_ms": 5000, - "harness_timeout_ms": 10000, - "default_mode": "manual", - "always_allow_seed": [], - "pending_timeout_ms": 1800000, - }), + // The shipped configuration defaults, straight from the source of + // truth so the published manifest can never drift from the worker. + default_config: crate::config::WorkerConfig::default().to_json(), supported_targets: vec![env!("TARGET").to_string()], } } diff --git a/approval-gate/src/pending.rs b/approval-gate/src/pending.rs index 89121d89c..52196a456 100644 --- a/approval-gate/src/pending.rs +++ b/approval-gate/src/pending.rs @@ -1,8 +1,7 @@ //! The pending inbox records: `approval_pending//`. //! Deliberately ephemeral — a record exists only while a call is held. -//! Every record has an explicit deletion path and the sweep as GC -//! backstop (approval-gate.md § State lifecycle), which is what keeps -//! `state::list` O(live holds). +//! Every record has an explicit deletion path (resolve, turn/session +//! purge), which is what keeps `state::list` O(live holds). use iii_sdk::{IIIError, III}; use serde_json::Value; @@ -18,8 +17,7 @@ pub fn pending_key(session_id: &str, function_call_id: &str) -> String { } /// Tolerant parse: null/garbage → None (a corrupt record must not wedge -/// the inbox; the sweep collects it once it has an `expires_at`, and a -/// record without one is skipped everywhere). +/// the inbox; turn/session purge collects orphaned records). pub fn parse_record(value: &Value) -> Option { if value.is_null() { return None; @@ -37,13 +35,11 @@ pub async fn get( iii: &III, session_id: &str, function_call_id: &str, - timeout_ms: u64, ) -> Result, IIIError> { let reply = state::get( iii, PENDING_SCOPE, &pending_key(session_id, function_call_id), - Some(timeout_ms), ) .await?; Ok(parse_record(&reply)) @@ -52,17 +48,12 @@ pub async fn get( /// Write the record. Returns the previous value when one existed (a /// concurrent duplicate hold lost the race — the caller must not emit a /// second `pending_created`). -pub async fn put( - iii: &III, - record: &PendingApprovalRecord, - timeout_ms: u64, -) -> Result, IIIError> { +pub async fn put(iii: &III, record: &PendingApprovalRecord) -> Result, IIIError> { let reply = state::set( iii, PENDING_SCOPE, &pending_key(&record.session_id, &record.function_call_id), serde_json::to_value(record).unwrap_or(Value::Null), - Some(timeout_ms), ) .await?; let old = reply.get("old_value").cloned().unwrap_or(Value::Null); @@ -72,8 +63,8 @@ pub async fn put( /// The single deletion helper every lifecycle path funnels through — /// deletion is the emit gate (approval-gate.md § Deletion is the emit /// gate): only the caller that observed the live record emits -/// `pending_resolved`, so concurrent paths (a resolve racing the sweep -/// racing a turn abort) produce exactly one event per record. +/// `pending_resolved`, so concurrent paths (a resolve racing a turn +/// abort) produce exactly one event per record. /// /// Mechanics: `state::set null` is the atomic gate — the engine swaps the /// value under its write lock and returns the prior one — but it stores a @@ -85,12 +76,11 @@ pub async fn delete_with_gate( iii: &III, session_id: &str, function_call_id: &str, - timeout_ms: u64, ) -> Result, IIIError> { let key = pending_key(session_id, function_call_id); - let reply = state::set(iii, PENDING_SCOPE, &key, Value::Null, Some(timeout_ms)).await?; + let reply = state::set(iii, PENDING_SCOPE, &key, Value::Null).await?; let old = reply.get("old_value").cloned().unwrap_or(Value::Null); - if let Err(e) = state::delete(iii, PENDING_SCOPE, &key, Some(timeout_ms)).await { + if let Err(e) = state::delete(iii, PENDING_SCOPE, &key).await { // The null tombstone survives until the next delete attempt; it // is invisible to readers (parse_record skips nulls). tracing::warn!(key, error = %e, "tombstone cleanup failed"); @@ -100,8 +90,8 @@ pub async fn delete_with_gate( /// Full-scope scan, values-only (the engine's `state::list` contract). /// Malformed/null values are skipped. -pub async fn list_all(iii: &III, timeout_ms: u64) -> Result, IIIError> { - let reply = state::list(iii, PENDING_SCOPE, Some(timeout_ms)).await?; +pub async fn list_all(iii: &III) -> Result, IIIError> { + let reply = state::list(iii, PENDING_SCOPE).await?; let values = match reply { Value::Array(items) => items, Value::Null => Vec::new(), @@ -126,7 +116,6 @@ mod tests { function_id: "shell::run".into(), arguments_excerpt: json!({ "cmd": "ls" }), pending_at: 100, - expires_at: 1_800_100, session_title: None, session_description: None, session_metadata: None, diff --git a/approval-gate/src/permissions/compile.rs b/approval-gate/src/permissions/compile.rs new file mode 100644 index 000000000..acb56ad85 --- /dev/null +++ b/approval-gate/src/permissions/compile.rs @@ -0,0 +1,219 @@ +use regex::Regex; + +use super::types::{Action, ConstraintSpec, RuleSpec}; + +#[derive(Debug, Clone)] +pub enum CompiledConstraint { + Equals(serde_json::Value), + Matches { regex: Regex, pattern: String }, +} + +#[derive(Debug, Clone)] +pub struct CompiledRule { + pub rule_id: String, + pub function_id: String, + pub glob: Option, + pub action: Action, + /// When set, the rule applies only in these session modes. + pub modes: Option>, + pub constraints: Vec<(String, CompiledConstraint)>, +} + +#[derive(Debug, thiserror::Error)] +pub enum CompileError { + #[error("rule {index}: {message}")] + Rule { index: usize, message: String }, +} + +fn compile_function_glob(pattern: &str) -> Result { + let mut re = String::from('^'); + for ch in pattern.chars() { + if ch == '*' { + re.push_str(".*"); + } else if ".^$|?+()[]{}\\".contains(ch) { + re.push('\\'); + re.push(ch); + } else { + re.push(ch); + } + } + re.push('$'); + Regex::new(&re).map_err(|e| CompileError::Rule { + index: 0, + message: format!("invalid glob {pattern:?}: {e}"), + }) +} + +fn compile_function_matcher( + pattern: &str, + index: usize, +) -> Result<(String, Option), CompileError> { + if !pattern.contains('*') { + return Ok((pattern.to_string(), None)); + } + let glob = compile_function_glob(pattern).map_err(|mut e| { + let CompileError::Rule { + index: _, + ref mut message, + } = &mut e; + *message = format!("rule {index}: {message}"); + e + })?; + Ok((pattern.to_string(), Some(glob))) +} + +pub fn match_function_id(rule: &CompiledRule, function_id: &str) -> bool { + match &rule.glob { + Some(glob) => glob.is_match(function_id), + None => rule.function_id == function_id, + } +} + +fn compile_constraint( + rule_id: &str, + field: &str, + spec: &ConstraintSpec, +) -> Result { + match spec { + ConstraintSpec::Equals(v) => Ok(CompiledConstraint::Equals(v.clone())), + ConstraintSpec::Matches(pattern) => { + let regex = Regex::new(pattern).map_err(|e| CompileError::Rule { + index: 0, + message: format!("rule {rule_id}: invalid regex {pattern:?} on field {field}: {e}"), + })?; + Ok(CompiledConstraint::Matches { + regex, + pattern: pattern.clone(), + }) + } + } +} + +pub fn compile_rule(spec: &RuleSpec, index: usize) -> Result { + match spec { + RuleSpec::Shorthand(raw) => { + if raw.starts_with('!') { + let function_id = raw.strip_prefix('!').unwrap_or(raw); + if function_id.is_empty() { + return Err(CompileError::Rule { + index, + message: "deny shorthand must be !".into(), + }); + } + let (function_id, glob) = compile_function_matcher(function_id, index)?; + return Ok(CompiledRule { + rule_id: function_id.clone(), + function_id, + glob, + action: Action::Deny, + modes: None, + constraints: Vec::new(), + }); + } + let (function_id, glob) = compile_function_matcher(raw, index)?; + Ok(CompiledRule { + rule_id: function_id.clone(), + function_id, + glob, + action: Action::Allow, + modes: None, + constraints: Vec::new(), + }) + } + RuleSpec::Structured { + rule_id, + function, + action, + modes, + args, + } => { + if function.is_empty() { + return Err(CompileError::Rule { + index, + message: "function is required".into(), + }); + } + let rid = rule_id + .as_deref() + .filter(|s| !s.is_empty()) + .unwrap_or(function.as_str()) + .to_string(); + let (function_id, glob) = compile_function_matcher(function, index)?; + let mut constraints: Vec<(String, CompiledConstraint)> = args + .iter() + .map(|(field, c)| compile_constraint(&rid, field, c).map(|cc| (field.clone(), cc))) + .collect::, _>>()?; + constraints.sort_by(|a, b| a.0.cmp(&b.0)); + Ok(CompiledRule { + rule_id: rid, + function_id, + glob, + action: *action, + modes: modes.clone(), + constraints, + }) + } + } +} + +fn args_object(args: &serde_json::Value) -> Option<&serde_json::Map> { + args.as_object() +} + +fn constraint_matches(actual: &serde_json::Value, c: &CompiledConstraint) -> bool { + match c { + CompiledConstraint::Equals(expected) => actual == expected, + CompiledConstraint::Matches { regex, .. } => { + actual.as_str().is_some_and(|s| regex.is_match(s)) + } + } +} + +fn matched_constraint(field: &str, c: &CompiledConstraint) -> crate::types::MatchedConstraint { + match c { + CompiledConstraint::Equals(v) => crate::types::MatchedConstraint { + field: field.to_string(), + operator: "equals".to_string(), + value: v.clone(), + }, + CompiledConstraint::Matches { pattern, .. } => crate::types::MatchedConstraint { + field: field.to_string(), + operator: "matches".to_string(), + value: serde_json::Value::String(pattern.clone()), + }, + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConstraintMatch { + Mismatch, + NoConstraint, + With, +} + +pub fn match_constraints( + args: &serde_json::Value, + constraints: &[(String, CompiledConstraint)], +) -> (ConstraintMatch, Option) { + if constraints.is_empty() { + return (ConstraintMatch::NoConstraint, None); + } + let Some(obj) = args_object(args) else { + return (ConstraintMatch::Mismatch, None); + }; + let mut matched: Option = None; + for (field, c) in constraints { + let actual = obj.get(field).unwrap_or(&serde_json::Value::Null); + if !constraint_matches(actual, c) { + return (ConstraintMatch::Mismatch, None); + } + if matched.is_none() { + matched = Some(matched_constraint(field, c)); + } + } + if matched.is_some() { + (ConstraintMatch::With, matched) + } else { + (ConstraintMatch::NoConstraint, None) + } +} diff --git a/approval-gate/src/permissions/default_rules.rs b/approval-gate/src/permissions/default_rules.rs new file mode 100644 index 000000000..190ed9b3d --- /dev/null +++ b/approval-gate/src/permissions/default_rules.rs @@ -0,0 +1,62 @@ +//! Built-in permission rules for step 5 of `approval::gate` when the +//! `approval-gate` configuration entry omits `rules` (or they fail to +//! compile). First match wins; no match → **hold**. +//! +//! The shipped defaults deny **only this worker's `approval::*` surface** +//! (the 12 registered functions — see `functions::catalog`). Every +//! other function_id holds until the operator adds custom `rules`. + +/// Shorthand rule strings compiled into [`super::default_permissions`]. +pub fn default_rule_strings() -> Vec<&'static str> { + vec!["!approval::*"] +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::functions::catalog; + use crate::permissions::{default_permissions, Decision}; + use crate::types::PermissionMode; + + fn catalog_function_ids() -> Vec<&'static str> { + catalog().iter().map(|s| s.function_id).collect() + } + + #[test] + fn default_rule_is_single_glob_over_registered_surface() { + assert_eq!(default_rule_strings(), ["!approval::*"]); + } + + #[test] + fn default_rules_deny_every_registered_function() { + let p = default_permissions(); + for fid in catalog_function_ids() { + assert!( + matches!( + p.check(fid, &serde_json::json!({}), PermissionMode::Manual), + Decision::Deny { .. } + ), + "{fid} should deny" + ); + } + } + + #[test] + fn default_rules_hold_other_workers() { + let p = default_permissions(); + for fid in [ + "shell::run", + "web::fetch", + "provider::anthropic::chat", + "configuration::set", + ] { + assert!( + matches!( + p.check(fid, &serde_json::json!({}), PermissionMode::Manual), + Decision::NeedsApproval + ), + "{fid} should hold" + ); + } + } +} diff --git a/approval-gate/src/permissions/mod.rs b/approval-gate/src/permissions/mod.rs new file mode 100644 index 000000000..d0b84480a --- /dev/null +++ b/approval-gate/src/permissions/mod.rs @@ -0,0 +1,296 @@ +//! Inline permission rule evaluation — ported from harness policy engine. +//! Rules live in the `approval-gate` configuration entry (`GateDefaults::rules`). + +mod compile; +mod default_rules; +mod types; + +pub use compile::{ + compile_rule, match_constraints, match_function_id, CompiledRule, ConstraintMatch, +}; +pub use types::{Action, ConstraintSpec, Decision, RuleSpec}; + +use compile::CompileError; +use serde_json::Value; +use tracing::warn; + +use crate::types::PermissionMode; + +pub fn rule_applies_in_mode(rule: &compile::CompiledRule, mode: PermissionMode) -> bool { + match &rule.modes { + None => true, + Some(modes) => modes.contains(&mode), + } +} + +/// Globs from allow rules explicitly scoped to auto mode — seeds per-session +/// `always_allow` on first mutation. +pub fn auto_allow_seed_from_specs(specs: &[RuleSpec]) -> Vec { + specs + .iter() + .filter_map(|spec| match spec { + RuleSpec::Structured { + function, + action: Action::Allow, + modes: Some(modes), + .. + } if modes.contains(&PermissionMode::Auto) => Some(function.clone()), + _ => None, + }) + .collect() +} + +#[derive(Debug, Clone)] +pub struct Permissions { + rules: Vec, +} + +impl Permissions { + pub fn empty() -> Self { + Self { rules: Vec::new() } + } + + pub fn rule_count(&self) -> usize { + self.rules.len() + } + + pub fn compile(specs: &[RuleSpec]) -> Result { + let mut rules = Vec::with_capacity(specs.len()); + let mut seen = std::collections::HashSet::new(); + for (idx, spec) in specs.iter().enumerate() { + let rule = compile_rule(spec, idx)?; + if !seen.insert(rule.rule_id.clone()) { + warn!(rule_id = %rule.rule_id, "duplicate rule_id in approval-gate rules"); + } + rules.push(rule); + } + Ok(Self { rules }) + } + + /// Tolerant compile: skip invalid entries; return empty on total failure. + pub fn compile_tolerant(specs: &[RuleSpec]) -> Self { + match Self::compile(specs) { + Ok(p) => p, + Err(e) => { + warn!(error = %e, "failed to compile permission rules; using empty policy"); + Self::empty() + } + } + } + + pub fn check(&self, function_id: &str, args: &Value, mode: PermissionMode) -> Decision { + for rule in &self.rules { + if !rule_applies_in_mode(rule, mode) { + continue; + } + if !match_function_id(rule, function_id) { + continue; + } + let (outcome, matched) = match_constraints(args, &rule.constraints); + if matches!(outcome, ConstraintMatch::Mismatch) { + continue; + } + return match rule.action { + Action::Allow => Decision::Allow { + rule_id: rule.rule_id.clone(), + }, + Action::Deny => Decision::Deny { + rule_id: rule.rule_id.clone(), + matched_constraint: matched, + }, + }; + } + Decision::NeedsApproval + } +} + +/// Parse one rule from a JSON config value. +pub fn parse_rule_spec(value: &Value) -> Option { + if let Some(s) = value.as_str() { + return Some(RuleSpec::Shorthand(s.to_string())); + } + let obj = value.as_object()?; + let function = obj.get("function")?.as_str()?.to_string(); + let action = match obj.get("action")?.as_str()? { + "allow" => Action::Allow, + "deny" => Action::Deny, + _ => return None, + }; + let rule_id = obj + .get("rule_id") + .and_then(Value::as_str) + .map(str::to_string); + let modes = obj.get("modes").and_then(|v| { + v.as_array().map(|items| { + items + .iter() + .filter_map(Value::as_str) + .filter_map(|s| match s { + "manual" => Some(PermissionMode::Manual), + "auto" => Some(PermissionMode::Auto), + "full" => Some(PermissionMode::Full), + _ => None, + }) + .collect::>() + }) + }); + let args = obj + .get("args") + .and_then(Value::as_object) + .map(|map| { + map.iter() + .filter_map(|(field, c)| parse_constraint(c).map(|spec| (field.clone(), spec))) + .collect() + }) + .unwrap_or_default(); + Some(RuleSpec::Structured { + rule_id, + function, + action, + modes, + args, + }) +} + +fn parse_constraint(value: &Value) -> Option { + let obj = value.as_object()?; + if let Some(v) = obj.get("equals") { + return Some(ConstraintSpec::Equals(v.clone())); + } + if let Some(s) = obj.get("matches").and_then(Value::as_str) { + return Some(ConstraintSpec::Matches(s.to_string())); + } + None +} + +pub fn parse_rules_from_config(value: &Value) -> Vec { + let Some(arr) = value.as_array() else { + return Vec::new(); + }; + arr.iter().filter_map(parse_rule_spec).collect() +} + +pub fn default_rule_specs() -> Vec { + default_rules::default_rule_strings() + .into_iter() + .map(|s| RuleSpec::Shorthand(s.to_string())) + .collect() +} + +pub fn default_permissions() -> Permissions { + Permissions::compile(&default_rule_specs()).expect("built-in rules compile") +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn kind(rules: &[&str], fid: &str, args: Value) -> Decision { + let specs: Vec = rules + .iter() + .map(|s| RuleSpec::Shorthand((*s).to_string())) + .collect(); + Permissions::compile(&specs) + .unwrap() + .check(fid, &args, PermissionMode::Manual) + } + + #[test] + fn empty_permissions_gates_every_call() { + assert!(matches!( + Permissions::empty().check("shell::exec", &json!({}), PermissionMode::Manual), + Decision::NeedsApproval + )); + } + + #[test] + fn bare_string_allow() { + let d = kind(&["shell::fs::ls"], "shell::fs::ls", json!({})); + assert_eq!( + d, + Decision::Allow { + rule_id: "shell::fs::ls".into() + } + ); + } + + #[test] + fn deny_shorthand() { + assert!(matches!( + kind(&["!state::set"], "state::set", json!({})), + Decision::Deny { .. } + )); + } + + #[test] + fn catch_all_star_allows() { + let d = kind(&["*"], "shell::exec", json!({ "command": "rm -rf /" })); + assert_eq!( + d, + Decision::Allow { + rule_id: "*".into() + } + ); + } + + #[test] + fn catch_all_after_deny_still_denies() { + assert!(matches!( + kind(&["!state::set", "*"], "state::set", json!({})), + Decision::Deny { .. } + )); + assert!(matches!( + kind(&["!state::set", "*"], "shell::exec", json!({})), + Decision::Allow { .. } + )); + } + + #[test] + fn glob_provider_needs_approval_by_default() { + assert!(matches!( + default_permissions().check( + "provider::anthropic::chat", + &json!({}), + PermissionMode::Manual + ), + Decision::NeedsApproval + )); + } + + #[test] + fn web_fetch_needs_approval_by_default() { + assert!(matches!( + default_permissions().check("web::fetch", &json!({}), PermissionMode::Manual), + Decision::NeedsApproval + )); + } + + #[test] + fn unknown_function_needs_approval() { + assert!(matches!( + default_permissions().check("shell::run", &json!({}), PermissionMode::Manual), + Decision::NeedsApproval + )); + } + + #[test] + fn structured_equals_constraint() { + let specs = vec![RuleSpec::Structured { + rule_id: None, + function: "f".into(), + action: Action::Allow, + modes: None, + args: vec![("n".into(), ConstraintSpec::Equals(json!(0)))], + }]; + let p = Permissions::compile(&specs).unwrap(); + assert!(matches!( + p.check("f", &json!({ "n": 0 }), PermissionMode::Manual), + Decision::Allow { .. } + )); + assert!(matches!( + p.check("f", &json!({ "n": "0" }), PermissionMode::Manual), + Decision::NeedsApproval + )); + } +} diff --git a/approval-gate/src/permissions/types.rs b/approval-gate/src/permissions/types.rs new file mode 100644 index 000000000..0c37f55ef --- /dev/null +++ b/approval-gate/src/permissions/types.rs @@ -0,0 +1,41 @@ +use serde_json::Value; + +use crate::types::{MatchedConstraint, PermissionMode}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Action { + Allow, + Deny, +} + +#[derive(Debug, Clone, PartialEq)] +pub enum Decision { + Allow { + rule_id: String, + }, + Deny { + rule_id: String, + matched_constraint: Option, + }, + NeedsApproval, +} + +/// One entry from the `rules` configuration array. +#[derive(Debug, Clone)] +pub enum RuleSpec { + Shorthand(String), + Structured { + rule_id: Option, + function: String, + action: Action, + /// When set, the rule applies only in these session modes. Omit for all modes. + modes: Option>, + args: Vec<(String, ConstraintSpec)>, + }, +} + +#[derive(Debug, Clone)] +pub enum ConstraintSpec { + Equals(Value), + Matches(String), +} diff --git a/approval-gate/src/policy.rs b/approval-gate/src/policy.rs deleted file mode 100644 index 2ebaf9d0a..000000000 --- a/approval-gate/src/policy.rs +++ /dev/null @@ -1,117 +0,0 @@ -//! `policy::check_permissions` client — the gate's yaml-rule fallback. -//! Reply mapping (approval-gate.md § Yaml policy dependency): -//! `allow`/`deny`/`needs_approval` map directly; **unparseable replies -//! degrade to needs_approval** (a human look is the safe reading of -//! "don't know") while **transport failures and timeouts fail closed** -//! to deny (`gate_unavailable`). - -use std::time::Duration; - -use iii_sdk::{TriggerRequest, III}; -use serde_json::{json, Value}; - -use crate::types::MatchedConstraint; - -#[derive(Debug, Clone, PartialEq)] -pub enum PolicyOutcome { - Allow, - Deny { - rule_id: String, - matched_constraint: Option, - }, - NeedsApproval, - Unavailable(String), -} - -pub async fn check(iii: &III, function_id: &str, args: &Value, timeout_ms: u64) -> PolicyOutcome { - let payload = json!({ "function_id": function_id, "args": args }); - // Belt and braces: the trigger timeout should bound the call, but a - // misbehaving transport must not stretch the hook past its budget. - let reply = tokio::time::timeout( - Duration::from_millis(timeout_ms), - iii.trigger(TriggerRequest { - function_id: "policy::check_permissions".into(), - payload, - action: None, - timeout_ms: Some(timeout_ms), - }), - ) - .await; - match reply { - Err(_elapsed) => PolicyOutcome::Unavailable("policy consult timed out".to_string()), - Ok(Err(e)) => PolicyOutcome::Unavailable(format!("policy unreachable: {e}")), - Ok(Ok(value)) => parse_reply(&value), - } -} - -pub fn parse_reply(value: &Value) -> PolicyOutcome { - match value.get("decision").and_then(Value::as_str) { - Some("allow") => PolicyOutcome::Allow, - Some("deny") => PolicyOutcome::Deny { - rule_id: value - .get("rule_id") - .and_then(Value::as_str) - .unwrap_or_default() - .to_string(), - matched_constraint: value - .get("matched_constraint") - .and_then(|v| serde_json::from_value(v.clone()).ok()), - }, - Some("needs_approval") => PolicyOutcome::NeedsApproval, - _ => PolicyOutcome::NeedsApproval, - } -} - -#[cfg(test)] -mod tests { - use super::*; - use serde_json::json; - - #[test] - fn parses_allow_deny_needs_approval() { - assert_eq!( - parse_reply(&json!({ "decision": "allow", "rule_id": "r1" })), - PolicyOutcome::Allow - ); - assert_eq!( - parse_reply(&json!({ - "decision": "deny", - "rule_id": "r2", - "matched_constraint": { "field": "cmd", "operator": "eq", "value": "rm" } - })), - PolicyOutcome::Deny { - rule_id: "r2".into(), - matched_constraint: Some(MatchedConstraint { - field: "cmd".into(), - operator: "eq".into(), - value: json!("rm"), - }), - } - ); - assert_eq!( - parse_reply(&json!({ "decision": "needs_approval" })), - PolicyOutcome::NeedsApproval - ); - } - - #[test] - fn unparseable_replies_degrade_to_needs_approval() { - assert_eq!(parse_reply(&json!("garbage")), PolicyOutcome::NeedsApproval); - assert_eq!(parse_reply(&Value::Null), PolicyOutcome::NeedsApproval); - assert_eq!( - parse_reply(&json!({ "decision": "maybe?" })), - PolicyOutcome::NeedsApproval - ); - } - - #[test] - fn deny_tolerates_missing_rule_id_and_bad_constraint() { - assert_eq!( - parse_reply(&json!({ "decision": "deny", "matched_constraint": "not-an-object" })), - PolicyOutcome::Deny { - rule_id: String::new(), - matched_constraint: None, - } - ); - } -} diff --git a/approval-gate/src/session.rs b/approval-gate/src/session.rs index ab37bcd33..0a76c57c9 100644 --- a/approval-gate/src/session.rs +++ b/approval-gate/src/session.rs @@ -3,12 +3,12 @@ use iii_sdk::{IIIError, TriggerRequest, III}; use serde_json::{json, Value}; -pub async fn get(iii: &III, session_id: &str, timeout_ms: Option) -> Result { +pub async fn get(iii: &III, session_id: &str) -> Result { iii.trigger(TriggerRequest { function_id: "session::get".into(), payload: json!({ "session_id": session_id }), action: None, - timeout_ms, + timeout_ms: None, }) .await } diff --git a/approval-gate/src/settings.rs b/approval-gate/src/settings.rs index 57a5bfaef..28af0007a 100644 --- a/approval-gate/src/settings.rs +++ b/approval-gate/src/settings.rs @@ -37,7 +37,7 @@ pub fn seeded_from(cfg: &WorkerConfig, granted_at: i64) -> ApprovalSettings { ApprovalSettings { mode: cfg.default_mode, always_allow: cfg - .always_allow_seed + .auto_allow_seed() .iter() .map(|function_id| AlwaysAllowEntry { function_id: function_id.clone(), @@ -63,12 +63,8 @@ pub fn effective( /// Hot-path read for the gate: any failure (state outage, absent record, /// garbage) degrades to `None` → configuration defaults. Safe because the /// default mode never widens beyond what the deployment configured. -pub async fn read_tolerant( - iii: &III, - session_id: &str, - timeout_ms: u64, -) -> Option { - let reply = state::get(iii, SETTINGS_SCOPE, session_id, Some(timeout_ms)).await; +pub async fn read_tolerant(iii: &III, session_id: &str) -> Option { + let reply = state::get(iii, SETTINGS_SCOPE, session_id).await; match reply { Ok(value) => parse_settings(&value), Err(e) => { @@ -83,9 +79,8 @@ pub async fn read_tolerant( pub async fn read_strict( iii: &III, session_id: &str, - timeout_ms: u64, ) -> Result, ApprovalError> { - let reply = state::get(iii, SETTINGS_SCOPE, session_id, Some(timeout_ms)) + let reply = state::get(iii, SETTINGS_SCOPE, session_id) .await .map_err(|e| ApprovalError::StateUnavailable(format!("settings read failed: {e}")))?; Ok(parse_settings(&reply)) @@ -100,7 +95,6 @@ pub async fn materialize_and( iii: &III, session_id: &str, cfg: &WorkerConfig, - timeout_ms: u64, mutate: F, ) -> Result where @@ -108,7 +102,7 @@ where { validate_id("session_id", session_id)?; let now = now_ms(); - let base = read_strict(iii, session_id, timeout_ms) + let base = read_strict(iii, session_id) .await? .unwrap_or_else(|| seeded_from(cfg, now)); let next = mutate(base, now); @@ -117,7 +111,6 @@ where SETTINGS_SCOPE, session_id, serde_json::to_value(&next).unwrap_or(Value::Null), - Some(timeout_ms), ) .await .map_err(|e| ApprovalError::StateUnavailable(format!("settings write failed: {e}")))?; @@ -126,9 +119,9 @@ where /// Drop the stored record (the session reverts to configuration /// defaults). Returns whether a record existed. -pub async fn clear(iii: &III, session_id: &str, timeout_ms: u64) -> Result { +pub async fn clear(iii: &III, session_id: &str) -> Result { validate_id("session_id", session_id)?; - let old = state::delete(iii, SETTINGS_SCOPE, session_id, Some(timeout_ms)) + let old = state::delete(iii, SETTINGS_SCOPE, session_id) .await .map_err(|e| ApprovalError::StateUnavailable(format!("settings delete failed: {e}")))?; Ok(!old.is_null()) @@ -173,8 +166,14 @@ mod tests { fn defaults_with_seed() -> WorkerConfig { WorkerConfig { default_mode: PermissionMode::Auto, - always_allow_seed: vec!["state::get".into(), "engine::functions::list".into()], - ..WorkerConfig::default() + rules: vec![ + json!({ "function": "state::get", "action": "allow", "modes": ["auto"] }), + json!({ + "function": "engine::functions::list", + "action": "allow", + "modes": ["auto"] + }), + ], } } diff --git a/approval-gate/src/state.rs b/approval-gate/src/state.rs index 1e0dcf837..0075c234e 100644 --- a/approval-gate/src/state.rs +++ b/approval-gate/src/state.rs @@ -1,61 +1,46 @@ //! Thin `state::get` / `state::set` / `state::delete` / `state::list` -//! wrappers around `iii.trigger()` (llm-router pattern). +//! wrappers around `iii.trigger()` (llm-router pattern). No explicit +//! timeout — every call uses the SDK default. use iii_sdk::{IIIError, TriggerRequest, III}; use serde_json::{json, Value}; -pub async fn get( - iii: &III, - scope: &str, - key: &str, - timeout_ms: Option, -) -> Result { +pub async fn get(iii: &III, scope: &str, key: &str) -> Result { iii.trigger(TriggerRequest { function_id: "state::get".into(), payload: json!({ "scope": scope, "key": key }), action: None, - timeout_ms, + timeout_ms: None, }) .await } -pub async fn set( - iii: &III, - scope: &str, - key: &str, - value: Value, - timeout_ms: Option, -) -> Result { +pub async fn set(iii: &III, scope: &str, key: &str, value: Value) -> Result { iii.trigger(TriggerRequest { function_id: "state::set".into(), payload: json!({ "scope": scope, "key": key, "value": value }), action: None, - timeout_ms, + timeout_ms: None, }) .await } -pub async fn delete( - iii: &III, - scope: &str, - key: &str, - timeout_ms: Option, -) -> Result { +pub async fn delete(iii: &III, scope: &str, key: &str) -> Result { iii.trigger(TriggerRequest { function_id: "state::delete".into(), payload: json!({ "scope": scope, "key": key }), action: None, - timeout_ms, + timeout_ms: None, }) .await } -pub async fn list(iii: &III, scope: &str, timeout_ms: Option) -> Result { +pub async fn list(iii: &III, scope: &str) -> Result { iii.trigger(TriggerRequest { function_id: "state::list".into(), payload: json!({ "scope": scope }), action: None, - timeout_ms, + timeout_ms: None, }) .await } diff --git a/approval-gate/src/testkit/engine.rs b/approval-gate/src/testkit/engine.rs index ecec92bc4..92f41cf19 100644 --- a/approval-gate/src/testkit/engine.rs +++ b/approval-gate/src/testkit/engine.rs @@ -11,7 +11,7 @@ use iii_sdk::{ use serde_json::{json, Value}; use tokio::sync::{OnceCell, RwLock}; -use crate::configuration::{self, ConfigCell, TriggerHandles}; +use crate::configuration::{self, ConfigCell}; use crate::events::{self, Emitter, PENDING_CREATED, PENDING_RESOLVED}; use crate::functions::{self, Deps}; @@ -181,58 +181,87 @@ pub async fn require_engine() -> Option { pub type CallLog = Arc>>; +/// How the booted stack's inline permission `rules` are set. The gate +/// evaluates `WorkerConfig::rules` directly (no policy worker), so a test +/// drives it by overriding that list in the config snapshot. #[derive(Clone)] -pub enum PolicyStub { - Reply(Value), - Error(String), - Absent, +pub enum RulesOverride { + /// Keep the shipped built-in rules seeded into the configuration entry. + Shipped, + /// No rules — every call falls through to `needs_approval`. + Empty, + /// An explicit rule list (string shorthands or rule objects). + Custom(Vec), } -impl Default for PolicyStub { - fn default() -> Self { - Self::Reply(json!({ "decision": "needs_approval" })) - } +#[derive(Clone)] +pub struct BootOpts { + pub rules: RulesOverride, + /// When true, spawn the real `harness` worker and skip the fake + /// `harness::function::resolve` stub. + pub real_harness: bool, } -#[derive(Clone, Default)] -pub struct BootOpts { - pub policy: PolicyStub, +impl Default for BootOpts { + fn default() -> Self { + Self::needs_approval() + } } impl BootOpts { + /// Empty rules: every call needs approval (the default test posture). pub fn needs_approval() -> Self { Self { - policy: PolicyStub::Reply(json!({ "decision": "needs_approval" })), + rules: RulesOverride::Empty, + real_harness: false, + } + } + + /// Like [`Self::needs_approval`] but boots the real harness worker. + pub fn needs_approval_with_harness() -> Self { + Self { + rules: RulesOverride::Empty, + real_harness: true, } } + /// Allow everything (`["*"]`). pub fn allow() -> Self { Self { - policy: PolicyStub::Reply(json!({ "decision": "allow" })), + rules: RulesOverride::Custom(vec![json!("*")]), + real_harness: false, } } - pub fn deny() -> Self { + /// Allow exactly one function id / glob. + pub fn allow_function(function_id: impl Into) -> Self { Self { - policy: PolicyStub::Reply(json!({ "decision": "deny", "rule_id": "test" })), + rules: RulesOverride::Custom(vec![json!(function_id.into())]), + real_harness: false, } } - pub fn policy_reply(value: Value) -> Self { + /// Deny exactly one function id / glob (`!`-prefixed shorthand). + pub fn deny_function(function_id: impl Into) -> Self { Self { - policy: PolicyStub::Reply(value), + rules: RulesOverride::Custom(vec![json!(format!("!{}", function_id.into()))]), + real_harness: false, } } - pub fn policy_error(message: impl Into) -> Self { + /// An explicit rule list. + pub fn with_rules(rules: Vec) -> Self { Self { - policy: PolicyStub::Error(message.into()), + rules: RulesOverride::Custom(rules), + real_harness: false, } } - pub fn no_policy() -> Self { + /// The shipped built-in rules from configuration defaults. + pub fn shipped_rules() -> Self { Self { - policy: PolicyStub::Absent, + rules: RulesOverride::Shipped, + real_harness: false, } } } @@ -250,6 +279,47 @@ pub struct TestStack { pub created: CallLog, /// `approval::pending-resolved` deliveries. pub resolved: CallLog, + harness_child: Option, +} + +impl Drop for TestStack { + fn drop(&mut self) { + if let Some(mut child) = self.harness_child.take() { + let _ = child.kill(); + let _ = child.wait(); + } + } +} + +fn spawn_harness_worker(engine_url: &str) -> Option { + let bin = std::env::var("CARGO_BIN_EXE_harness").ok()?; + std::process::Command::new(bin) + .arg("--url") + .arg(engine_url) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .ok() +} + +async fn wait_for_harness(iii: &III) -> bool { + let deadline = Instant::now() + Duration::from_secs(15); + while Instant::now() < deadline { + if iii + .trigger(TriggerRequest { + function_id: "harness::status".into(), + payload: json!({ "session_id": "__probe__" }), + action: None, + timeout_ms: Some(500), + }) + .await + .is_ok() + { + return true; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + false } pub fn log_push(log: &CallLog, value: Value) { @@ -271,9 +341,17 @@ pub async fn boot(engine: &Engine, opts: BootOpts) -> TestStack { configuration::register_config(&iii, None) .await .expect("register approval-gate configuration schema"); - let cfg = configuration::fetch_config(&iii) + let mut cfg = configuration::fetch_config(&iii) .await .expect("fetch approval-gate configuration"); + // Drive the gate's inline permission rules from the boot options. The + // gate compiles `WorkerConfig::rules` per call, so overriding the list + // here is all a test needs to choose allow / deny / hold behaviour. + match &opts.rules { + RulesOverride::Shipped => {} + RulesOverride::Empty => cfg.rules = Vec::new(), + RulesOverride::Custom(rules) => cfg.rules = rules.clone(), + } let config: ConfigCell = Arc::new(RwLock::new(Arc::new(cfg))); let sets = events::register_trigger_types(&iii); @@ -284,31 +362,8 @@ pub async fn boot(engine: &Engine, opts: BootOpts) -> TestStack { }); functions::register_all(&iii, &deps); - let policy = opts.policy; - match policy { - PolicyStub::Reply(decision) => { - iii.register_function( - "policy::check_permissions", - RegisterFunction::new_async(move |_req: Value| { - let decision = decision.clone(); - async move { Ok::<_, iii_sdk::IIIError>(decision) } - }), - ); - } - PolicyStub::Error(message) => { - iii.register_function( - "policy::check_permissions", - RegisterFunction::new_async(move |_req: Value| { - let message = message.clone(); - async move { Err::(iii_sdk::IIIError::Handler(message)) } - }), - ); - } - PolicyStub::Absent => {} - } - let harness_calls: CallLog = Arc::default(); - { + if !opts.real_harness { let log = harness_calls.clone(); iii.register_function( "harness::function::resolve", @@ -381,17 +436,24 @@ pub async fn boot(engine: &Engine, opts: BootOpts) -> TestStack { }) .expect("bind pending_resolved"); + configuration::bind_hook(&iii); + // The typed, re-fetching config-change handler + its `configuration` - // trigger binding (mirrors the production boot order: last). The test - // engine has no harness hook / cron to bind, so the re-bindable handles - // start empty; reactive reload of the per-call knobs still applies. - let handles = Arc::new(TriggerHandles { - hook: Mutex::new(None), - sweep: Mutex::new(None), - }); - configuration::register_config_trigger(&iii, config.clone(), handles) + // trigger binding (mirrors the production boot order: last). + configuration::register_config_trigger(&iii, config.clone()) .expect("register configuration change trigger"); + let harness_child = if opts.real_harness { + let child = spawn_harness_worker(&engine.url).expect("spawn harness worker"); + assert!( + wait_for_harness(&iii).await, + "harness worker did not become ready" + ); + Some(child) + } else { + None + }; + TestStack { iii, deps, @@ -399,6 +461,7 @@ pub async fn boot(engine: &Engine, opts: BootOpts) -> TestStack { harness_calls, created, resolved, + harness_child, } } diff --git a/approval-gate/src/testkit/mod.rs b/approval-gate/src/testkit/mod.rs index a9864b48b..c099094db 100644 --- a/approval-gate/src/testkit/mod.rs +++ b/approval-gate/src/testkit/mod.rs @@ -4,6 +4,5 @@ pub mod engine; pub use engine::{ boot, call, engine_bin, hook_input, log_push, log_snapshot, require_engine, settle, - spawn_engine, state_get, state_set, wait_for, with_stack, BootOpts, CallLog, Engine, - PolicyStub, TestStack, + spawn_engine, state_get, state_set, wait_for, with_stack, BootOpts, CallLog, Engine, TestStack, }; diff --git a/approval-gate/src/types.rs b/approval-gate/src/types.rs index f69419076..4dbcaa2b1 100644 --- a/approval-gate/src/types.rs +++ b/approval-gate/src/types.rs @@ -64,7 +64,7 @@ pub enum PermissionMode { #[serde(rename_all = "snake_case")] pub enum GrantedBy { UserClick, - /// Copied from the deployment's `always_allow_seed` on first mutation. + /// Copied from deployment auto-scoped allow rules on first mutation. Seed, } @@ -152,8 +152,6 @@ pub struct PendingApprovalRecord { pub arguments_excerpt: Value, /// ms epoch pub pending_at: i64, - /// `pending_at + pending_timeout_ms` - pub expires_at: i64, // Denormalized session context — soft-fetched via session::get at // hold time; omitted when the fetch fails or session-manager is absent. @@ -181,7 +179,6 @@ pub struct PendingApprovalRecord { pub enum ResolvedOutcome { Allow, Deny, - Timeout, Aborted, } @@ -242,7 +239,7 @@ pub struct HookInput { pub enum HookOutput { Continue, Deny { reason: String }, - Hold { pending_timeout_ms: i64 }, + Hold, } // --------------------------------------------------------------------------- @@ -412,11 +409,8 @@ mod tests { json!({ "decision": "deny", "reason": "nope" }) ); assert_eq!( - serde_json::to_value(HookOutput::Hold { - pending_timeout_ms: 1_800_000 - }) - .unwrap(), - json!({ "decision": "hold", "pending_timeout_ms": 1_800_000 }) + serde_json::to_value(HookOutput::Hold).unwrap(), + json!({ "decision": "hold" }) ); } @@ -489,7 +483,6 @@ mod tests { function_id: "shell::run".into(), arguments_excerpt: json!({ "cmd": "ls" }), pending_at: 100, - expires_at: 200, session_title: None, session_description: None, session_metadata: None, diff --git a/approval-gate/tests/contract_parity.rs b/approval-gate/tests/contract_parity.rs new file mode 100644 index 000000000..66123c0bc --- /dev/null +++ b/approval-gate/tests/contract_parity.rs @@ -0,0 +1,37 @@ +//! Cross-worker wire-shape locks between approval-gate and harness v1. + +use serde_json::json; + +#[test] +fn gate_hold_matches_harness_parse_output() { + let hold = json!({ "decision": "hold" }); + assert!(hold.get("pending_timeout_ms").is_none()); +} + +#[test] +fn resolve_allow_payload_matches_harness_function_resolve_request() { + let payload = json!({ + "session_id": "s_1", + "turn_id": "t_1", + "function_call_id": "c_1", + "action": "execute", + }); + assert_eq!(payload["action"], json!("execute")); + assert!(payload.get("content").is_none()); + assert!(payload.get("is_error").is_none()); +} + +#[test] +fn resolve_deny_payload_matches_harness_function_resolve_request() { + let payload = json!({ + "session_id": "s_1", + "turn_id": "t_1", + "function_call_id": "c_1", + "action": "deliver", + "is_error": true, + "content": [{ "type": "text", "text": "denied" }], + "details": { "status": "denied", "denied_by": "user" }, + }); + assert_eq!(payload["action"], json!("deliver")); + assert_eq!(payload["is_error"], json!(true)); +} diff --git a/approval-gate/tests/golden/schemas/approval.add-always-allow.json b/approval-gate/tests/golden/schemas/approval.add-always-allow.json index 633adf081..e3786465b 100644 --- a/approval-gate/tests/golden/schemas/approval.add-always-allow.json +++ b/approval-gate/tests/golden/schemas/approval.add-always-allow.json @@ -87,7 +87,7 @@ "type": "string" }, { - "description": "Copied from the deployment's `always_allow_seed` on first mutation.", + "description": "Copied from deployment auto-scoped allow rules on first mutation.", "enum": [ "seed" ], diff --git a/approval-gate/tests/golden/schemas/approval.approve-always.json b/approval-gate/tests/golden/schemas/approval.approve-always.json index 5f14af499..78efd165a 100644 --- a/approval-gate/tests/golden/schemas/approval.approve-always.json +++ b/approval-gate/tests/golden/schemas/approval.approve-always.json @@ -87,7 +87,7 @@ "type": "string" }, { - "description": "Copied from the deployment's `always_allow_seed` on first mutation.", + "description": "Copied from deployment auto-scoped allow rules on first mutation.", "enum": [ "seed" ], diff --git a/approval-gate/tests/golden/schemas/approval.gate.json b/approval-gate/tests/golden/schemas/approval.gate.json index 8b2761b30..fe7515fab 100644 --- a/approval-gate/tests/golden/schemas/approval.gate.json +++ b/approval-gate/tests/golden/schemas/approval.gate.json @@ -121,15 +121,10 @@ "hold" ], "type": "string" - }, - "pending_timeout_ms": { - "format": "int64", - "type": "integer" } }, "required": [ - "decision", - "pending_timeout_ms" + "decision" ], "type": "object" } diff --git a/approval-gate/tests/golden/schemas/approval.get-pending.json b/approval-gate/tests/golden/schemas/approval.get-pending.json index 73b1350f6..b1c003eee 100644 --- a/approval-gate/tests/golden/schemas/approval.get-pending.json +++ b/approval-gate/tests/golden/schemas/approval.get-pending.json @@ -63,11 +63,6 @@ "format": "int64", "type": "integer" }, - "expires_at": { - "description": "`pending_at + pending_timeout_ms`", - "format": "int64", - "type": "integer" - }, "function_call_id": { "type": "string" }, @@ -107,7 +102,6 @@ } }, "required": [ - "expires_at", "function_call_id", "function_id", "pending_at", diff --git a/approval-gate/tests/golden/schemas/approval.get-settings.json b/approval-gate/tests/golden/schemas/approval.get-settings.json index 557271cba..c8b6f6b1a 100644 --- a/approval-gate/tests/golden/schemas/approval.get-settings.json +++ b/approval-gate/tests/golden/schemas/approval.get-settings.json @@ -83,7 +83,7 @@ "type": "string" }, { - "description": "Copied from the deployment's `always_allow_seed` on first mutation.", + "description": "Copied from deployment auto-scoped allow rules on first mutation.", "enum": [ "seed" ], diff --git a/approval-gate/tests/golden/schemas/approval.list-pending.json b/approval-gate/tests/golden/schemas/approval.list-pending.json index d2c91a83e..e46ae5ffe 100644 --- a/approval-gate/tests/golden/schemas/approval.list-pending.json +++ b/approval-gate/tests/golden/schemas/approval.list-pending.json @@ -65,11 +65,6 @@ "format": "int64", "type": "integer" }, - "expires_at": { - "description": "`pending_at + pending_timeout_ms`", - "format": "int64", - "type": "integer" - }, "function_call_id": { "type": "string" }, @@ -109,7 +104,6 @@ } }, "required": [ - "expires_at", "function_call_id", "function_id", "pending_at", diff --git a/approval-gate/tests/golden/schemas/approval.pending-created.json b/approval-gate/tests/golden/schemas/approval.pending-created.json index 77672cb33..84ec00d3f 100644 --- a/approval-gate/tests/golden/schemas/approval.pending-created.json +++ b/approval-gate/tests/golden/schemas/approval.pending-created.json @@ -21,11 +21,6 @@ "format": "int64", "type": "integer" }, - "expires_at": { - "description": "`pending_at + pending_timeout_ms`", - "format": "int64", - "type": "integer" - }, "function_call_id": { "type": "string" }, @@ -65,7 +60,6 @@ } }, "required": [ - "expires_at", "function_call_id", "function_id", "pending_at", diff --git a/approval-gate/tests/golden/schemas/approval.pending-resolved.json b/approval-gate/tests/golden/schemas/approval.pending-resolved.json index 064da58ec..fa23ee937 100644 --- a/approval-gate/tests/golden/schemas/approval.pending-resolved.json +++ b/approval-gate/tests/golden/schemas/approval.pending-resolved.json @@ -7,7 +7,6 @@ "enum": [ "allow", "deny", - "timeout", "aborted" ], "type": "string" diff --git a/approval-gate/tests/golden/schemas/approval.remove-always-allow.json b/approval-gate/tests/golden/schemas/approval.remove-always-allow.json index e0799fbcf..d6eab4278 100644 --- a/approval-gate/tests/golden/schemas/approval.remove-always-allow.json +++ b/approval-gate/tests/golden/schemas/approval.remove-always-allow.json @@ -87,7 +87,7 @@ "type": "string" }, { - "description": "Copied from the deployment's `always_allow_seed` on first mutation.", + "description": "Copied from deployment auto-scoped allow rules on first mutation.", "enum": [ "seed" ], diff --git a/approval-gate/tests/golden/schemas/approval.set-mode.json b/approval-gate/tests/golden/schemas/approval.set-mode.json index c5bbab019..47fde3552 100644 --- a/approval-gate/tests/golden/schemas/approval.set-mode.json +++ b/approval-gate/tests/golden/schemas/approval.set-mode.json @@ -97,7 +97,7 @@ "type": "string" }, { - "description": "Copied from the deployment's `always_allow_seed` on first mutation.", + "description": "Copied from deployment auto-scoped allow rules on first mutation.", "enum": [ "seed" ], diff --git a/approval-gate/tests/golden/schemas/approval.sweep.json b/approval-gate/tests/golden/schemas/approval.sweep.json deleted file mode 100644 index 9ee87f416..000000000 --- a/approval-gate/tests/golden/schemas/approval.sweep.json +++ /dev/null @@ -1,26 +0,0 @@ -{ - "description": "Internal: cron handler (expire pending records past expires_at).", - "function_id": "approval::sweep", - "request_schema": { - "$schema": "http://json-schema.org/draft-07/schema#", - "description": "Input of `approval::sweep` — the cron tick carries no arguments. A struct (not `Value`) keeps the request schema concrete; unknown fields (the cron trigger payload, engine-injected ids) are ignored.", - "title": "SweepRequest", - "type": "object" - }, - "response_schema": { - "$schema": "http://json-schema.org/draft-07/schema#", - "description": "Output of `approval::sweep`: how many expired records were collected.", - "properties": { - "swept": { - "format": "uint", - "minimum": 0.0, - "type": "integer" - } - }, - "required": [ - "swept" - ], - "title": "SweepResponse", - "type": "object" - } -} diff --git a/approval-gate/tests/harness_integration.rs b/approval-gate/tests/harness_integration.rs new file mode 100644 index 000000000..999e58797 --- /dev/null +++ b/approval-gate/tests/harness_integration.rs @@ -0,0 +1,226 @@ +//! Cross-worker integration: real `harness` + in-process approval-gate on a +//! live engine. Self-skips when `iii` or the harness binary is unavailable. + +use std::collections::BTreeMap; + +use approval_gate::testkit::{ + boot, call, engine_bin, hook_input, spawn_engine, state_get, state_set, BootOpts, +}; +use iii_sdk::RegisterFunction; +use serde_json::json; + +async fn with_harness_stack(opts: BootOpts, f: F) +where + F: FnOnce(approval_gate::testkit::TestStack) -> Fut, + Fut: std::future::Future, +{ + if engine_bin().is_none() { + eprintln!("skipping: no iii engine"); + return; + } + let Some(engine) = spawn_engine().await else { + eprintln!("skipping: failed to spawn engine"); + return; + }; + if std::env::var("CARGO_BIN_EXE_harness").is_err() { + eprintln!("skipping: harness binary not built for integration tests"); + return; + } + let stack = boot(&engine, opts).await; + f(stack).await; +} + +async fn seed_held_turn( + stack: &approval_gate::testkit::TestStack, + session_id: &str, + call_id: &str, +) { + let mut calls = BTreeMap::new(); + calls.insert( + call_id.to_string(), + json!({ + "state": "pending", + "function_id": "shell::run", + "held_by": "approval::gate", + "pending_timeout_ms": null, + "pending_at": 1, + }), + ); + let record = json!({ + "turn_id": "t_1", + "session_id": session_id, + "status": "awaiting_functions", + "step": 1, + "turn_count": 0, + "depth": 0, + "abort": false, + "options": { + "model": "test", + "max_turns": 8, + "output": { "type": "text" }, + "functions": { "allow": ["*"], "expose": "agent_trigger" }, + "max_validation_retries": 2, + }, + "calls": calls, + }); + state_set(&stack.iii, "harness_turn", session_id, record).await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn harness_held_call_survives_sweep_pending() { + with_harness_stack( + BootOpts::needs_approval_with_harness(), + |stack| async move { + seed_held_turn(&stack, "s_hold", "c_hold").await; + + let swept = call(&stack.iii, "harness::sweep-pending", json!({})) + .await + .expect("sweep"); + assert_eq!(swept["resolved"], json!(0)); + + let turn = state_get(&stack.iii, "harness_turn", "s_hold").await; + assert_eq!(turn["calls"]["c_hold"]["state"], json!("pending")); + }, + ) + .await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn harness_pre_trigger_hold_then_resolve_allow() { + with_harness_stack(BootOpts::needs_approval_with_harness(), |stack| async move { + let iii = &stack.iii; + + iii.register_function( + "shell::run", + RegisterFunction::new_async(|_req: serde_json::Value| async { + Ok::<_, iii_sdk::IIIError>(json!({ "ok": true })) + }), + ); + + let turn = json!({ + "turn_id": "t_1", + "session_id": "s_1", + "status": "running", + "step": 1, + "turn_count": 0, + "depth": 0, + "abort": false, + "options": { + "model": "test", + "max_turns": 8, + "output": { "type": "text" }, + "functions": { "allow": ["*"], "expose": "agent_trigger" }, + "max_validation_retries": 2, + }, + "calls": {}, + }); + state_set(iii, "harness_turn", "s_1", turn).await; + + let gate = call( + iii, + "approval::gate", + hook_input("s_1", "c_1", "shell::run"), + ) + .await + .expect("gate"); + assert_eq!(gate["decision"], json!("hold")); + + let pending = call( + iii, + "harness::function::trigger", + json!({ + "session_id": "s_1", + "call": { "id": "c_1", "function_id": "shell::run", "arguments": { "cmd": "echo hi" } } + }), + ) + .await + .expect("trigger"); + assert_eq!(pending["pending"], json!(true)); + assert!(pending.get("pending_timeout_ms").is_none()); + + let res = call( + iii, + "approval::resolve", + json!({ "session_id": "s_1", "function_call_id": "c_1", "decision": "allow" }), + ) + .await + .expect("resolve"); + assert_eq!(res["resolved"], json!(true)); + }) + .await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn resolve_deny_delivers_is_error_through_harness() { + with_harness_stack( + BootOpts::needs_approval_with_harness(), + |stack| async move { + let iii = &stack.iii; + + call( + iii, + "approval::gate", + hook_input("s_2", "c_deny", "shell::run"), + ) + .await + .expect("gate hold"); + + let res = call( + iii, + "approval::resolve", + json!({ + "session_id": "s_2", + "function_call_id": "c_deny", + "decision": "deny", + "reason": "nope" + }), + ) + .await + .expect("resolve"); + assert_eq!(res["resolved"], json!(true)); + + let turn = json!({ + "turn_id": "t_1", + "session_id": "s_2", + "status": "awaiting_functions", + "step": 1, + "turn_count": 0, + "depth": 0, + "abort": false, + "options": { + "model": "test", + "max_turns": 8, + "output": { "type": "text" }, + "functions": { "allow": ["*"], "expose": "agent_trigger" }, + "max_validation_retries": 2, + }, + "calls": { + "c_deny": { + "state": "pending", + "function_id": "shell::run", + "held_by": "approval::gate", + } + }, + }); + state_set(iii, "harness_turn", "s_2", turn).await; + + let deliver = call( + iii, + "harness::function::resolve", + json!({ + "session_id": "s_2", + "turn_id": "t_1", + "function_call_id": "c_deny", + "action": "deliver", + "is_error": true, + "content": [{ "type": "text", "text": "denied" }], + "details": { "status": "denied", "denied_by": "user" }, + }), + ) + .await + .expect("harness resolve deliver"); + assert_eq!(deliver["resolved"], json!(true)); + }, + ) + .await; +} diff --git a/approval-gate/tests/integration.rs b/approval-gate/tests/integration.rs index 60ad0ea5c..a9c8942b2 100644 --- a/approval-gate/tests/integration.rs +++ b/approval-gate/tests/integration.rs @@ -21,6 +21,7 @@ async fn hold_writes_record_emits_once_and_is_idempotent() { .await .expect("gate call"); assert_eq!(out["decision"], json!("hold")); + assert!(out.get("pending_timeout_ms").is_none()); let record = call( iii, @@ -62,6 +63,7 @@ async fn resolve_allow_releases_and_deny_delivers_through_the_fake_harness() { .await .unwrap(); assert_eq!(out["decision"], json!("hold")); + assert!(out.get("pending_timeout_ms").is_none()); } let res = call( @@ -108,48 +110,6 @@ async fn resolve_allow_releases_and_deny_delivers_through_the_fake_harness() { .await; } -#[tokio::test(flavor = "multi_thread")] -async fn sweep_expires_records_and_emits_timeout_exactly_once() { - with_stack(BootOpts::needs_approval(), |stack| async move { - let iii = &stack.iii; - - call( - iii, - "state::set", - json!({ "scope": "approval_pending", "key": "s_9/c_9", "value": { - "session_id": "s_9", - "turn_id": "t_9", - "function_call_id": "c_9", - "function_id": "shell::run", - "arguments_excerpt": {}, - "pending_at": 100, - "expires_at": 200, - "depth": 0, - }}), - ) - .await - .unwrap(); - - let swept = call(iii, "approval::sweep", json!({})).await.unwrap(); - assert_eq!(swept["swept"], json!(1)); - - let harness = log_snapshot(&stack.harness_calls); - assert_eq!(harness.len(), 1); - assert_eq!(harness[0]["details"]["status"], json!("timeout")); - - let swept = call(iii, "approval::sweep", json!({})).await.unwrap(); - assert_eq!(swept["swept"], json!(0)); - - assert!( - wait_for(3_000, || !log_snapshot(&stack.resolved).is_empty()).await, - "timeout event should reach the recorder" - ); - settle().await; - assert_eq!(log_snapshot(&stack.resolved).len(), 1); - }) - .await; -} - #[tokio::test(flavor = "multi_thread")] async fn configuration_set_reloads_defaults_reactively() { with_stack(BootOpts::needs_approval(), |stack| async move { @@ -163,6 +123,7 @@ async fn configuration_set_reloads_defaults_reactively() { .await .unwrap(); assert_eq!(out["decision"], json!("hold")); + assert!(out.get("pending_timeout_ms").is_none()); call( iii, diff --git a/approval-gate/tests/schemas.rs b/approval-gate/tests/schemas.rs index 326ca0523..b3b400eb2 100644 --- a/approval-gate/tests/schemas.rs +++ b/approval-gate/tests/schemas.rs @@ -46,7 +46,7 @@ fn trigger_spec_to_pretty_json(spec: &TriggerSpec) -> String { pretty } -/// The catalog must cover exactly the 13 registered functions, in +/// The catalog must cover exactly the 12 registered functions, in /// registration order (kept in lockstep with `register_all`). The internal /// `approval::on-config-change` handler is registered separately via /// `configuration::register_config_trigger` (mirroring session-manager / @@ -69,7 +69,6 @@ fn catalog_lists_all_functions_in_registration_order() { "approval::clear-settings", "approval::on-session-deleted", "approval::on-turn-completed", - "approval::sweep", ] ); } diff --git a/console/web/src/components/permissions/DefaultPermissionModePicker.tsx b/console/web/src/components/permissions/DefaultPermissionModePicker.tsx index 28feefb35..14a662dbf 100644 --- a/console/web/src/components/permissions/DefaultPermissionModePicker.tsx +++ b/console/web/src/components/permissions/DefaultPermissionModePicker.tsx @@ -1,10 +1,6 @@ import { useState } from 'react' import { ModeToggle } from '@/components/ui/ModeToggle' -import { - loadDefaultPermissionMode, - type PermissionMode, - saveDefaultPermissionMode, -} from '@/lib/storage' +import { type PermissionMode, saveDefaultPermissionMode } from '@/lib/storage' import { FullModeConfirmDialog } from './FullModeConfirmDialog' interface DefaultPermissionModePickerProps { @@ -14,24 +10,23 @@ interface DefaultPermissionModePickerProps { } /** - * User-level default mode applied only to NEW conversations. Existing - * conversations own their own mode independently after creation. - * - * Selecting Full opens a confirmation dialog before localStorage is - * written. Cancel keeps the previous value. + * User-level default mode applied only to NEW conversations. When `value` and + * `onChange` are supplied the parent owns persistence (approval-gate config). */ export function DefaultPermissionModePicker({ value, onChange, }: DefaultPermissionModePickerProps) { const [internal, setInternal] = useState( - () => value ?? loadDefaultPermissionMode(), + () => value ?? 'manual', ) const [pendingFull, setPendingFull] = useState(false) const current = value ?? internal function commit(next: PermissionMode) { - saveDefaultPermissionMode(next) + if (!onChange) { + saveDefaultPermissionMode(next) + } setInternal(next) onChange?.(next) } diff --git a/console/web/src/hooks/use-approval-settings.ts b/console/web/src/hooks/use-approval-settings.ts index 25454b69a..a07f436c4 100644 --- a/console/web/src/hooks/use-approval-settings.ts +++ b/console/web/src/hooks/use-approval-settings.ts @@ -1,4 +1,5 @@ import { useCallback, useEffect, useRef, useState } from 'react' +import { loadApprovalGateDefaults } from '@/lib/backend/approval-gate-config' import { type ApprovalSettings, DEFAULT_APPROVAL_SETTINGS, @@ -9,7 +10,6 @@ import { removeAlwaysAllow as rpcRemoveAlwaysAllow, setApprovalMode, } from '@/lib/backend/approval-settings' -import { loadDefaultAllowlist, loadDefaultPermissionMode } from '@/lib/storage' interface UseApprovalSettingsResult { settings: ApprovalSettings @@ -60,16 +60,22 @@ export function useApprovalSettings( // conversations already in flight. const isFirstTouch = fetched.mode_set_at === 0 if (isFirstTouch) { - const userDefault = loadDefaultPermissionMode() - const defaultAllowlist = loadDefaultAllowlist() let next = fetched - if (userDefault !== 'manual') { - next = await setApprovalMode(sessionId, userDefault) - if (cancelled || activeRef.current !== sessionId) return - } - for (const functionId of defaultAllowlist) { - next = await rpcAddAlwaysAllow(sessionId, functionId) - if (cancelled || activeRef.current !== sessionId) return + try { + const deployment = await loadApprovalGateDefaults() + if (deployment.defaultMode !== 'manual') { + next = await setApprovalMode(sessionId, deployment.defaultMode) + if (cancelled || activeRef.current !== sessionId) return + } + for (const functionId of deployment.allowlist) { + next = await rpcAddAlwaysAllow(sessionId, functionId) + if (cancelled || activeRef.current !== sessionId) return + } + } catch (err) { + console.error( + '[approval-settings] deployment defaults unavailable; seeding manual mode', + err, + ) } setSettings(next) } else { diff --git a/console/web/src/lib/backend/approval-gate-config.test.ts b/console/web/src/lib/backend/approval-gate-config.test.ts new file mode 100644 index 000000000..89eca856c --- /dev/null +++ b/console/web/src/lib/backend/approval-gate-config.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest' +import { + autoAllowSeedFromRules, + deriveFunctionPolicy, +} from './approval-gate-config' + +describe('approval-gate-config', () => { + it('extracts auto-scoped allow rules as the seed allowlist', () => { + expect( + autoAllowSeedFromRules([ + { function: 'state::get', action: 'allow', modes: ['auto'] }, + { function: 'shell::run', action: 'allow' }, + '!approval::*', + ]), + ).toEqual(['state::get']) + }) + + it('derives harness deny globs from deployment rules', () => { + expect( + deriveFunctionPolicy([ + { function: 'shell::run', action: 'deny' }, + '!state::set', + ]), + ).toEqual({ + allow: ['*'], + deny: ['approval::*', 'configuration::*', 'shell::run', 'state::set'], + expose: 'agent_trigger', + }) + }) +}) diff --git a/console/web/src/lib/backend/approval-gate-config.ts b/console/web/src/lib/backend/approval-gate-config.ts new file mode 100644 index 000000000..4a5e01050 --- /dev/null +++ b/console/web/src/lib/backend/approval-gate-config.ts @@ -0,0 +1,134 @@ +/** + * Read/write deployment permission defaults from the canonical + * `approval-gate` configuration entry (single source — no localStorage). + */ + +import type { PermissionMode } from '@/lib/backend/approval-settings' +import type { HarnessFunctionPolicy } from '@/lib/backend/harness-send' +import { + getConfiguration, + type JsonValue, + setConfiguration, +} from '@/pages/Configuration/tabs/WorkersTab/api' + +export const APPROVAL_GATE_CONFIG_ID = 'approval-gate' + +export interface ApprovalGateConfigView { + default_mode: PermissionMode + rules: JsonValue[] +} + +interface StructuredRule { + function?: string + action?: string + modes?: string[] +} + +function isPermissionMode(v: unknown): v is PermissionMode { + return v === 'manual' || v === 'auto' || v === 'full' +} + +function asRulesArray(value: JsonValue | undefined): JsonValue[] { + return Array.isArray(value) ? value : [] +} + +/** Extract auto-mode trust globs from mode-scoped allow rules. */ +export function autoAllowSeedFromRules(rules: JsonValue[]): string[] { + const out: string[] = [] + for (const entry of rules) { + if (typeof entry === 'string') continue + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) continue + const rule = entry as StructuredRule + if (rule.action !== 'allow') continue + if (!rule.modes?.includes('auto')) continue + if (typeof rule.function === 'string' && rule.function.length > 0) { + out.push(rule.function) + } + } + return out.sort() +} + +/** Derive the harness structural floor from deployment rules. */ +export function deriveFunctionPolicy( + rules: JsonValue[], +): HarnessFunctionPolicy { + const deny = new Set(['approval::*', 'configuration::*']) + for (const entry of rules) { + if (typeof entry === 'string') { + if (entry.startsWith('!')) deny.add(entry.slice(1)) + continue + } + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) continue + const rule = entry as StructuredRule + if (rule.action === 'deny' && typeof rule.function === 'string') { + deny.add(rule.function) + } + } + return { + allow: ['*'], + deny: Array.from(deny).sort(), + expose: 'agent_trigger', + } +} + +export async function loadApprovalGateConfig(): Promise { + const raw = await getConfiguration(APPROVAL_GATE_CONFIG_ID) + const obj = + raw && typeof raw === 'object' && !Array.isArray(raw) + ? (raw as Record) + : {} + const default_mode = isPermissionMode(obj.default_mode) + ? obj.default_mode + : 'manual' + return { + default_mode, + rules: asRulesArray(obj.rules), + } +} + +function withoutAutoSeedRules(rules: JsonValue[]): JsonValue[] { + return rules.filter((entry) => { + if (typeof entry === 'string') return true + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return true + const rule = entry as StructuredRule + if (rule.action !== 'allow') return true + return !(rule.modes?.length === 1 && rule.modes[0] === 'auto') + }) +} + +/** Persist default mode + auto allowlist into the deployment rules list. */ +export async function saveApprovalGateDefaults( + defaultMode: PermissionMode, + allowlist: string[], +): Promise { + const current = await loadApprovalGateConfig() + const baseRules = withoutAutoSeedRules(current.rules) + const seedRules: JsonValue[] = allowlist.map((function_id) => ({ + function: function_id, + action: 'allow', + modes: ['auto'], + })) + const nextRules = [...baseRules, ...seedRules] + const payload = { + default_mode: defaultMode, + rules: nextRules, + } + await setConfiguration({ id: APPROVAL_GATE_CONFIG_ID, value: payload }) + return { + default_mode: defaultMode, + rules: nextRules, + } +} + +export async function loadApprovalGateDefaults(): Promise<{ + defaultMode: PermissionMode + allowlist: string[] + functionPolicy: HarnessFunctionPolicy +}> { + const cfg = await loadApprovalGateConfig() + return { + defaultMode: cfg.default_mode, + allowlist: autoAllowSeedFromRules(cfg.rules), + functionPolicy: deriveFunctionPolicy(cfg.rules), + } +} diff --git a/console/web/src/lib/backend/real.ts b/console/web/src/lib/backend/real.ts index abe0712ca..b77b3d5d8 100644 --- a/console/web/src/lib/backend/real.ts +++ b/console/web/src/lib/backend/real.ts @@ -1,7 +1,7 @@ /** - * iii-browser-sdk + harness turn kickoff. Permissions live in the harness's - * iii-permissions.yaml; the console ships the per-mode system prompt and the - * dispatch policy on each send. + * iii-browser-sdk + harness turn kickoff. Deployment permission rules live in + * the `approval-gate` configuration entry; the console derives the harness + * structural floor from those rules on each send. * * Transcript content (tokens, message snapshots, function-call cards, results) * renders from session-manager events reconciled by the conversations layer @@ -21,6 +21,7 @@ import { listPendingApprovals, startApprovalEventsSubscription, } from './approval-events-live' +import { loadApprovalGateDefaults } from './approval-gate-config' import { getTurnStatus, type HarnessFunctionPolicy, @@ -50,11 +51,12 @@ interface RunParams { /** * The chat composer is a general-purpose agent surface: expose the whole bus - * via `agent_trigger`. The harness's `iii-permissions.yaml` and the - * approval-gate remain the safety layer (deny-by-default plumbing, human gate). + * via `agent_trigger`. The approval-gate rules supply the structural floor; + * the gate hook remains the human decision surface. */ -const CHAT_FUNCTION_POLICY: HarnessFunctionPolicy = { +const FALLBACK_FUNCTION_POLICY: HarnessFunctionPolicy = { allow: ['*'], + deny: ['approval::*', 'configuration::*'], expose: 'agent_trigger', } @@ -152,6 +154,18 @@ async function* realStream( const thinkingLevel = toThinkingLevel(opts?.thinkingLevel) + let functionPolicy = FALLBACK_FUNCTION_POLICY + try { + functionPolicy = (await loadApprovalGateDefaults()).functionPolicy + } catch (err) { + if (import.meta.env.DEV) { + console.warn( + '[real-backend] approval-gate config unavailable; using fallback policy', + err, + ) + } + } + // Kick off (or steer) the turn. A turn already running for this session // folds the message in (merge) rather than rejecting — no busy error. void sendTurn(client, { @@ -163,7 +177,7 @@ async function* realStream( session: { metadata: { surface: 'console' } }, options: { system_prompt: buildModeSystemPrompt(mode, provider), - functions: CHAT_FUNCTION_POLICY, + functions: functionPolicy, ...(thinkingLevel ? { thinking_level: thinkingLevel } : {}), metadata: { session_id: sessionId, message_id: messageId }, }, diff --git a/console/web/src/pages/Configuration/tabs/ConsoleSettingsTab.tsx b/console/web/src/pages/Configuration/tabs/ConsoleSettingsTab.tsx index 68417240f..314bc39d1 100644 --- a/console/web/src/pages/Configuration/tabs/ConsoleSettingsTab.tsx +++ b/console/web/src/pages/Configuration/tabs/ConsoleSettingsTab.tsx @@ -1,4 +1,4 @@ -import { useCallback, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useState } from 'react' import { DefaultPermissionModePicker } from '@/components/permissions/DefaultPermissionModePicker' import { FunctionAllowlistTree } from '@/components/permissions/FunctionAllowlistTree' import { @@ -11,13 +11,13 @@ import { ModeToggle } from '@/components/ui/ModeToggle' import { useFunctionsCatalog } from '@/hooks/use-functions-catalog' import type { Theme } from '@/hooks/use-theme' import { getDefaultBackend } from '@/lib/backend' +import { + autoAllowSeedFromRules, + loadApprovalGateDefaults, + saveApprovalGateDefaults, +} from '@/lib/backend/approval-gate-config' import type { PermissionMode } from '@/lib/backend/approval-settings' import { filterAllowlistCandidates } from '@/lib/permissions/allowlist-filter' -import { - loadDefaultAllowlist, - loadDefaultPermissionMode, - saveDefaultAllowlist, -} from '@/lib/storage' // Provider credentials + settings now live in the llm-router `configuration` // entry, edited via the schema-driven form on the workers tab. @@ -41,34 +41,82 @@ export function ConsoleSettingsTab({ theme, onThemeChange, }: ConsoleSettingsTabProps) { - // Controlled default-permission-mode + per-user allowlist. Both back to - // localStorage. The allowlist section only renders while mode === 'auto' - // (it has no effect under manual/full, so showing it would mislead). - const [defaultMode, setDefaultMode] = useState(() => - loadDefaultPermissionMode(), + // Deployment defaults from the approval-gate configuration entry (single source). + const [defaultMode, setDefaultMode] = useState('manual') + const [allowlist, setAllowlist] = useState([]) + const [loaded, setLoaded] = useState(false) + + useEffect(() => { + let cancelled = false + void (async () => { + try { + const cfg = await loadApprovalGateDefaults() + if (cancelled) return + setDefaultMode(cfg.defaultMode) + setAllowlist(cfg.allowlist) + } catch (err) { + console.error( + '[console-settings] failed to load approval-gate config', + err, + ) + } finally { + if (!cancelled) setLoaded(true) + } + })() + return () => { + cancelled = true + } + }, []) + + const persistDefaults = useCallback( + async (mode: PermissionMode, list: string[]) => { + try { + const next = await saveApprovalGateDefaults(mode, list) + setDefaultMode(next.default_mode) + setAllowlist(autoAllowSeedFromRules(next.rules)) + } catch (err) { + console.error( + '[console-settings] failed to save approval-gate config', + err, + ) + } + }, + [], ) - const [allowlist, setAllowlist] = useState(() => - loadDefaultAllowlist(), + + const handleModeChange = useCallback( + (next: PermissionMode) => { + setDefaultMode(next) + void persistDefaults(next, allowlist) + }, + [allowlist, persistDefaults], ) - const allowlistSet = useMemo(() => new Set(allowlist), [allowlist]) - const addAllow = useCallback((functionId: string) => { - setAllowlist((prev) => { - if (prev.includes(functionId)) return prev - const next = [...prev, functionId] - saveDefaultAllowlist(next) - return next - }) - }, []) + const addAllow = useCallback( + (functionId: string) => { + setAllowlist((prev) => { + if (prev.includes(functionId)) return prev + const next = [...prev, functionId] + void persistDefaults(defaultMode, next) + return next + }) + }, + [defaultMode, persistDefaults], + ) - const removeAllow = useCallback((functionId: string) => { - setAllowlist((prev) => { - if (!prev.includes(functionId)) return prev - const next = prev.filter((id) => id !== functionId) - saveDefaultAllowlist(next) - return next - }) - }, []) + const removeAllow = useCallback( + (functionId: string) => { + setAllowlist((prev) => { + if (!prev.includes(functionId)) return prev + const next = prev.filter((id) => id !== functionId) + void persistDefaults(defaultMode, next) + return next + }) + }, + [defaultMode, persistDefaults], + ) + + const allowlistSet = useMemo(() => new Set(allowlist), [allowlist]) const { functionEntries } = useFunctionsCatalog(getDefaultBackend().id) const allowlistCandidates = useMemo( @@ -102,14 +150,14 @@ export function ConsoleSettingsTab({
} meta="manual prompts for everything · auto skips functions on your allowlist · full skips everything" diff --git a/harness/src/deferred.rs b/harness/src/deferred.rs index 13aee5fc2..4afcb943e 100644 --- a/harness/src/deferred.rs +++ b/harness/src/deferred.rs @@ -244,6 +244,24 @@ pub async fn resolve_parent( } } +/// Whether a pending checkpoint should be resolved by the expiry sweep. +/// Approval/hook holds (`held_by`) never expire; sub-agent child pendings do. +fn pending_call_expired( + cp: &crate::types::turn::CallCheckpoint, + default_timeout_ms: u64, + now: i64, +) -> bool { + if cp.state != CallState::Pending { + return false; + } + if cp.held_by.is_some() { + return false; + } + let timeout = cp.pending_timeout_ms.unwrap_or(default_timeout_ms); + let pending_at = cp.pending_at.unwrap_or(now); + now.saturating_sub(pending_at) as u64 >= timeout +} + /// Resolve every pending call past its `pending_timeout_ms` with an error. pub async fn sweep_expired(deps: &Deps) -> Result { let cfg = deps.cfg().await; @@ -257,14 +275,7 @@ pub async fn sweep_expired(deps: &Deps) -> Result { continue; } for (call_id, cp) in &record.calls { - if cp.state != CallState::Pending { - continue; - } - let timeout = cp - .pending_timeout_ms - .unwrap_or(cfg.default_pending_timeout_ms); - let pending_at = cp.pending_at.unwrap_or(now); - if now.saturating_sub(pending_at) as u64 >= timeout { + if pending_call_expired(cp, cfg.default_pending_timeout_ms, now) { expired.push(( record.session_id.clone(), record.turn_id.clone(), @@ -308,3 +319,64 @@ fn render_text(value: &Value) -> String { other => serde_json::to_string(other).unwrap_or_default(), } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::types::turn::{CallCheckpoint, CallState}; + + fn cp( + state: CallState, + held_by: Option<&str>, + timeout_ms: Option, + pending_at: i64, + ) -> CallCheckpoint { + CallCheckpoint { + state, + function_id: Some("shell::run".into()), + entry_id: None, + child_session_id: if held_by.is_none() && timeout_ms.is_some() { + Some("child".into()) + } else { + None + }, + child_turn_id: if held_by.is_none() && timeout_ms.is_some() { + Some("t_child".into()) + } else { + None + }, + held_by: held_by.map(str::to_string), + pending_timeout_ms: timeout_ms, + pending_at: Some(pending_at), + } + } + + #[test] + fn approval_hold_never_expires() { + let now = 1_000_000; + let checkpoint = cp( + CallState::Pending, + Some("approval::gate"), + None, + now - 999_999, + ); + assert!(!pending_call_expired(&checkpoint, 1_800_000, now)); + } + + #[test] + fn sub_agent_pending_expires_past_wait_guard() { + let now = 1_000_000; + let fresh = cp(CallState::Pending, None, Some(60_000), now - 30_000); + assert!(!pending_call_expired(&fresh, 1_800_000, now)); + let stale = cp(CallState::Pending, None, Some(60_000), now - 60_000); + assert!(pending_call_expired(&stale, 1_800_000, now)); + } + + #[test] + fn sub_agent_pending_uses_default_timeout_when_unset() { + let now = 1_000_000; + let default = 1_800_000; + let stale = cp(CallState::Pending, None, None, now - default as i64); + assert!(pending_call_expired(&stale, default, now)); + } +} diff --git a/harness/src/functions/function_trigger.rs b/harness/src/functions/function_trigger.rs index fc4414c3e..36c9fe7a2 100644 --- a/harness/src/functions/function_trigger.rs +++ b/harness/src/functions/function_trigger.rs @@ -119,14 +119,12 @@ pub async fn handle( elapsed(started), )); } - PreTriggerOutcome::Hold { - pending_timeout_ms, .. - } => { + PreTriggerOutcome::Hold { .. } => { return Ok(FunctionTriggerResponse::Pending(TriggerPendingResponse { function_call_id: req.call.id, function_id: req.call.function_id, pending: true, - pending_timeout_ms, + pending_timeout_ms: None, })); } } diff --git a/harness/src/hooks/runner.rs b/harness/src/hooks/runner.rs index e481395c4..d5565e1ef 100644 --- a/harness/src/hooks/runner.rs +++ b/harness/src/hooks/runner.rs @@ -29,7 +29,7 @@ struct HookMutations { enum HookOutcome { Continue(HookMutations), Deny(String), - Hold(Option), + Hold, } /// Outcome of the `pre_generate` chain. @@ -50,7 +50,6 @@ pub enum PreTriggerOutcome { }, Deny(String), Hold { - pending_timeout_ms: Option, held_by: String, annotations: Map, }, @@ -78,7 +77,7 @@ impl HookRegistry { match self.invoke(&binding, input).await { HookOutcome::Continue(_) => {} HookOutcome::Deny(reason) => return Err(reason), - HookOutcome::Hold(_) => {} + HookOutcome::Hold => {} } } Ok(()) @@ -115,7 +114,7 @@ impl HookRegistry { merge(&mut annotations, m.annotations); } HookOutcome::Deny(reason) => return PreGenerateOutcome::Deny(reason), - HookOutcome::Hold(_) => {} + HookOutcome::Hold => {} } } PreGenerateOutcome::Continue { @@ -164,9 +163,8 @@ impl HookRegistry { merge(&mut annotations, m.annotations); } HookOutcome::Deny(reason) => return PreTriggerOutcome::Deny(reason), - HookOutcome::Hold(pending_timeout_ms) => { + HookOutcome::Hold => { return PreTriggerOutcome::Hold { - pending_timeout_ms, held_by: binding.function_id.clone(), annotations, } @@ -265,7 +263,7 @@ fn parse_output(value: Value) -> HookOutcome { .unwrap_or("denied by hook") .to_string(), ), - Some("hold") => HookOutcome::Hold(value.get("pending_timeout_ms").and_then(Value::as_u64)), + Some("hold") => HookOutcome::Hold, _ => { let mut muts = HookMutations::default(); if let Some(m) = value.get("mutations") { @@ -341,10 +339,15 @@ mod tests { HookOutcome::Deny(r) => assert_eq!(r, "nope"), _ => panic!("expected deny"), } - match parse_output(json!({ "decision": "hold", "pending_timeout_ms": 1000 })) { - HookOutcome::Hold(Some(t)) => assert_eq!(t, 1000), + match parse_output(json!({ "decision": "hold" })) { + HookOutcome::Hold => {} _ => panic!("expected hold"), } + // Legacy hooks may still send pending_timeout_ms; it is ignored. + assert!(matches!( + parse_output(json!({ "decision": "hold", "pending_timeout_ms": 1000 })), + HookOutcome::Hold + )); } #[test] diff --git a/harness/src/turn_loop.rs b/harness/src/turn_loop.rs index 715756aac..7a7c3f89a 100644 --- a/harness/src/turn_loop.rs +++ b/harness/src/turn_loop.rs @@ -350,15 +350,9 @@ pub async fn run_step( crate::state::put_turn(&deps.iii, &record, cfg.session_timeout_ms).await?; continue; } - crate::hooks::runner::PreTriggerOutcome::Hold { - pending_timeout_ms, - held_by, - .. - } => { + crate::hooks::runner::PreTriggerOutcome::Hold { held_by, .. } => { let info = trigger::PendingInfo { - pending_timeout_ms: Some( - pending_timeout_ms.unwrap_or(cfg.default_pending_timeout_ms), - ), + pending_timeout_ms: None, held_by: Some(held_by), child_session_id: None, child_turn_id: None, diff --git a/iii-permissions.yaml b/iii-permissions.yaml index 97d58db85..702a39ded 100644 --- a/iii-permissions.yaml +++ b/iii-permissions.yaml @@ -12,7 +12,6 @@ version: 1 rules: # Gate, state, auth, routing — agents must not call these directly. - '!approval::resolve' - - '!policy::check_permissions' - '!hook-fanout::publish_collect' - '!state::set' - '!state::update'