diff --git a/.sqlx/query-1dd2e60c7b39789db4fd126784dccc7a27080a5f7dea5d42af0afc77b77877db.json b/.sqlx/query-0390276a145cf74b5d8e3e2f85b2e6056319d5d685559a97f5d43e02ec73d8e5.json similarity index 72% rename from .sqlx/query-1dd2e60c7b39789db4fd126784dccc7a27080a5f7dea5d42af0afc77b77877db.json rename to .sqlx/query-0390276a145cf74b5d8e3e2f85b2e6056319d5d685559a97f5d43e02ec73d8e5.json index 11eb06866..82b241c85 100644 --- a/.sqlx/query-1dd2e60c7b39789db4fd126784dccc7a27080a5f7dea5d42af0afc77b77877db.json +++ b/.sqlx/query-0390276a145cf74b5d8e3e2f85b2e6056319d5d685559a97f5d43e02ec73d8e5.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT default_deferred_execution, default_agent_self_setup, call_timeout_ms, max_call_timeout_ms\n FROM orgs WHERE id = $1 FOR UPDATE", + "query": "SELECT default_deferred_execution, default_agent_self_setup, call_timeout_ms, max_call_timeout_ms,\n default_remember_ttl_secs\n FROM orgs WHERE id = $1 FOR UPDATE", "describe": { "columns": [ { @@ -46,6 +46,17 @@ "name": "max_call_timeout_ms" } } + }, + { + "ordinal": 4, + "name": "default_remember_ttl_secs", + "type_info": "Int4", + "origin": { + "Table": { + "table": "orgs", + "name": "default_remember_ttl_secs" + } + } } ], "parameters": { @@ -57,8 +68,9 @@ false, false, true, + true, true ] }, - "hash": "1dd2e60c7b39789db4fd126784dccc7a27080a5f7dea5d42af0afc77b77877db" + "hash": "0390276a145cf74b5d8e3e2f85b2e6056319d5d685559a97f5d43e02ec73d8e5" } diff --git a/.sqlx/query-73d37304206f85ae5a92dbdadaf954e0ab2f4c556a857feb98adae6c78f5192c.json b/.sqlx/query-73d37304206f85ae5a92dbdadaf954e0ab2f4c556a857feb98adae6c78f5192c.json new file mode 100644 index 000000000..67c63c90f --- /dev/null +++ b/.sqlx/query-73d37304206f85ae5a92dbdadaf954e0ab2f4c556a857feb98adae6c78f5192c.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT default_remember_ttl_secs FROM orgs WHERE id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "default_remember_ttl_secs", + "type_info": "Int4", + "origin": { + "Table": { + "table": "orgs", + "name": "default_remember_ttl_secs" + } + } + } + ], + "parameters": { + "Left": [ + "Uuid" + ] + }, + "nullable": [ + true + ] + }, + "hash": "73d37304206f85ae5a92dbdadaf954e0ab2f4c556a857feb98adae6c78f5192c" +} diff --git a/.sqlx/query-260a1ab8de1ea4997f5802ca889105e22ac0c8da565d0f7941f498d66d7262fd.json b/.sqlx/query-e3412d889cff77af86f7441ec2abbabd9b87734dc6b5798d6a5493599b83be5e.json similarity index 65% rename from .sqlx/query-260a1ab8de1ea4997f5802ca889105e22ac0c8da565d0f7941f498d66d7262fd.json rename to .sqlx/query-e3412d889cff77af86f7441ec2abbabd9b87734dc6b5798d6a5493599b83be5e.json index cca348da9..8cf4e6a4c 100644 --- a/.sqlx/query-260a1ab8de1ea4997f5802ca889105e22ac0c8da565d0f7941f498d66d7262fd.json +++ b/.sqlx/query-e3412d889cff77af86f7441ec2abbabd9b87734dc6b5798d6a5493599b83be5e.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "UPDATE orgs\n SET default_deferred_execution = $2,\n default_agent_self_setup = $3,\n call_timeout_ms = $4,\n max_call_timeout_ms = $5,\n updated_at = now()\n WHERE id = $1", + "query": "UPDATE orgs\n SET default_deferred_execution = $2,\n default_agent_self_setup = $3,\n call_timeout_ms = $4,\n max_call_timeout_ms = $5,\n default_remember_ttl_secs = $6,\n updated_at = now()\n WHERE id = $1", "describe": { "columns": [], "parameters": { @@ -9,10 +9,11 @@ "Bool", "Bool", "Int4", + "Int4", "Int4" ] }, "nullable": [] }, - "hash": "260a1ab8de1ea4997f5802ca889105e22ac0c8da565d0f7941f498d66d7262fd" + "hash": "e3412d889cff77af86f7441ec2abbabd9b87734dc6b5798d6a5493599b83be5e" } diff --git a/DECISIONS.md b/DECISIONS.md index cc9e61b60..28f6b3bdd 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1542,3 +1542,9 @@ The disclosure input parses a form's body as JSON, as it does for a JSON body. **Date**: 2026-10-27 **Decision**: `GET /health` returns 503 `status: degraded` when its bounded `SELECT 1` fails, and 200 `status: ok` otherwise. `/ready` keeps the same gate. The Cloud Run startup and liveness probes on the API are `tcp_socket` probes on the container port, not HTTP probes on `/health`. The GCM uptime check behind `[P0] API Down` and the Better Stack monitor stay on `/health`, so a database outage pages in prod. Dev's nightly Cloud SQL stop therefore fires dev's `API Down` every night, and that is accepted: it emails only, and it proves the path end to end. `routes/health.rs`, `infra/modules/cloud-run/main.tf`. **Rationale**: `/health` was always-200 so that a Cloud SQL blip would not make the probes restart containers. That never worked. The probe's default 1 s timeout is shorter than `/health`'s 2 s database timeout, so with the database gone the liveness probe failed anyway. Dev's instance was shut down about 90 s after every nightly stop, and a replacement cannot boot without the database (`create_app` connects and migrates eagerly), so Cloud Run answered 500 until morning. The design gave neither a surviving container nor an honest status. Splitting the two questions fixes both. Whether the process is up is a TCP connect, which nothing upstream can fail. Whether the API can serve is `/health`, which the monitors read and which no longer restarts anything. A TCP startup probe gives up nothing, because the listener binds only after `create_app` has connected, migrated and preflighted. No new endpoint was added, since a TCP probe needs none. + +## D-NEXT: "Allow & Remember" expires after an org-set default, 7 days out of the box; an omitted `ttl` means that default, not forever + +**Date**: 2026-10-28 +**Decision**: Orgs have a `default_remember_ttl_secs` setting (migration 139). It is nullable, NULL means forever, it is bounded to 60 s to 365 d, and the column default is 604800 (7 days). The column default also backfills every existing org. Admins set it on `PATCH /v1/orgs/{id}/execution-settings`, and Org Settings shows it as a select below the call timeouts. When `POST /v1/approvals/{id}/resolve` gets `allow_remember` with no `ttl`, the rule expires `now + default`. A permanent rule now needs an explicit `ttl: "forever"`, which `/resolve` accepts (it used to 400 on it). The dashboard's Allow split button still starts on Once (#844), tags the org default "· default" in its duration menu, and always sends `ttl`. The MCP remember dialog preselects the org default; if that default is not one of the dialog's menu values, it is added as a `"{n}s"` option, and the dialog forwards `forever` verbatim. The SDK approval card gets an "Org default" choice, selected by default, that leaves `ttl` out. One parser, `overslash_core::types::duration::parse_rule_ttl`, serves `/resolve` and `PATCH /v1/permissions/{id}`. +**Rationale**: Before this, a remembered rule was permanent unless the reviewer changed the picker. "Never" was the preselection, and leaving the field out meant forever. So the cheapest action created the longest-lived grant. The org default had to be applied on the server, when `ttl` is missing, rather than only preselected in the dashboard, because API, MCP and SDK resolvers never see the dashboard. This changes behaviour for API callers that relied on omitting `ttl` to get forever. That trade is deliberate: any caller that wants a permanent grant can still ask for one, and now has to ask explicitly. Backfilling existing orgs instead of grandfathering them to forever is also deliberate, because the safer default should reach the orgs that never chose. Rules already written keep the expiry they were written with. diff --git a/SCHEMA.sql b/SCHEMA.sql index c0a006796..a1e9a9a2d 100644 --- a/SCHEMA.sql +++ b/SCHEMA.sql @@ -2,7 +2,7 @@ -- PostgreSQL database dump -- -\restrict IU7gGYzzNTd60ELJcMnX68xk6MwIMaSwIk56fgOPGg3Phe56vVK1sb2CM7pgn1c +\restrict Ima6egLcgrnJ79TnxwblysVaIC9DjWdCHPDn0FnJXFFtcuWvCqfvYirq9Tx4HXW -- Dumped from database version 16.14 (Debian 16.14-1.pgdg12+1) -- Dumped by pg_dump version 16.15 (Ubuntu 16.15-0ubuntu0.24.04.1) @@ -1064,9 +1064,11 @@ CREATE TABLE public.orgs ( call_timeout_ms integer, max_call_timeout_ms integer, default_agent_self_setup boolean DEFAULT true NOT NULL, + default_remember_ttl_secs integer DEFAULT 604800, CONSTRAINT orgs_approval_auto_bubble_secs_check CHECK ((approval_auto_bubble_secs >= 0)), CONSTRAINT orgs_audit_response_body_mode_check CHECK ((audit_response_body_mode = ANY (ARRAY['off'::text, 'errors_only'::text, 'all'::text]))), CONSTRAINT orgs_call_timeout_bounds CHECK ((((call_timeout_ms IS NULL) OR ((call_timeout_ms >= 1000) AND (call_timeout_ms <= 600000))) AND ((max_call_timeout_ms IS NULL) OR ((max_call_timeout_ms >= 1000) AND (max_call_timeout_ms <= 600000))) AND ((call_timeout_ms IS NULL) OR (max_call_timeout_ms IS NULL) OR (call_timeout_ms <= max_call_timeout_ms)))), + CONSTRAINT orgs_default_remember_ttl_bounds CHECK (((default_remember_ttl_secs IS NULL) OR ((default_remember_ttl_secs >= 60) AND (default_remember_ttl_secs <= 31536000)))), CONSTRAINT orgs_plan_check CHECK ((plan = ANY (ARRAY['standard'::text, 'free_unlimited'::text, 'trial'::text]))), CONSTRAINT orgs_user_template_policy_check CHECK ((user_template_policy = ANY (ARRAY['none'::text, 'restrictive'::text, 'full'::text]))) ); @@ -1107,6 +1109,13 @@ COMMENT ON COLUMN public.orgs.max_call_timeout_ms IS 'Ceiling on any resolved ca COMMENT ON COLUMN public.orgs.default_agent_self_setup IS 'When true (default), a newly-created first-level agent (depth = 1) is seeded with the four overslash:*_own self-setup permission rules: manage_services_own, manage_templates_own, manage_connections_own, request_secrets_own. Existing agents are not touched when this flips.'; +-- +-- Name: COLUMN orgs.default_remember_ttl_secs; Type: COMMENT; Schema: public; Owner: - +-- + +COMMENT ON COLUMN public.orgs.default_remember_ttl_secs IS 'Default expiry in seconds for rules written by Allow & Remember when the resolver does not pass a ttl. NULL = forever.'; + + -- -- Name: pending_checkouts; Type: TABLE; Schema: public; Owner: - -- @@ -4046,5 +4055,5 @@ ALTER TABLE ONLY public.webhook_subscriptions -- PostgreSQL database dump complete -- -\unrestrict IU7gGYzzNTd60ELJcMnX68xk6MwIMaSwIk56fgOPGg3Phe56vVK1sb2CM7pgn1c +\unrestrict Ima6egLcgrnJ79TnxwblysVaIC9DjWdCHPDn0FnJXFFtcuWvCqfvYirq9Tx4HXW diff --git a/SPEC.md b/SPEC.md index b03b15584..c22eb6c84 100644 --- a/SPEC.md +++ b/SPEC.md @@ -497,7 +497,7 @@ The flow above expanded as discrete steps: 2. **Group check (Layer 1)**: is the service + access level within the owner-user's group grants? If not → **deny** (not approvable) 3. **Auto-approve bypass**: if the matching grant's `auto_approve_level` permits this action's risk, skip Layer 2 and call immediately (a deny rule still applies to mutating calls) 4. **Permission key check (Layer 2)**: are all derived keys covered by existing rules for this identity? If yes → **auto-approve** -5. If not → **create approval request** → user decides → "Allow & Remember" stores keys with optional TTL +5. If not → **create approval request** → user decides → "Allow & Remember" stores keys with a TTL (the org default — 7 days unless changed — or `forever` when asked for explicitly) ### Hierarchical Resolution @@ -609,6 +609,8 @@ Approval and secret requests are **not notified immediately**. Only requests tha "Allow & Remember" on an approval creates permission key rules with optional TTL. These rules auto-approve matching future requests. Permission rules and remembered approvals are the same concept — "permission rules" is the storage format, "remembered approvals" is the user-facing term. Users can view and revoke them per identity via the dashboard. +**Expiry.** The resolver may pass `ttl` (`1h`, `7d`, … up to 365 days, or `forever`). An omitted `ttl` takes the org's `default_remember_ttl_secs` (`PATCH /v1/orgs/{id}/execution-settings`; NULL = forever; **default 7 days**). The MCP remember dialog preselects that default; the dashboard's Allow menu tags it (the button itself starts on Once). Changing it never touches rules already written. + The rule is stored **only after a successful `POST /v1/approvals/{id}/call`** — a cancelled, expired, or failed replay leaves no rule behind. This prevents a reviewer from being silently committed to auto-approving an action they never saw succeed. ### Replay Semantics diff --git a/UI_SPEC.md b/UI_SPEC.md index d00ab800c..b64cd1437 100644 --- a/UI_SPEC.md +++ b/UI_SPEC.md @@ -294,7 +294,7 @@ A button with two halves joined seamlessly — left side is the default action, **Structure**: left half has rounded left corners, right half (▾) has rounded right corners. No gap between them — they share a continuous border. -**Approval variant**: Left = runs the selected duration, labelled "Allow once" / "Allow for 24 hours" / "Allow always". Right dropdown = duration menu: **Once** (plain allow), then an "Allow & remember" section with **For 1 hour / 24 hours / 7 days / 30 days** and **Always** (remember at the scope chosen in the action bar's "Remember as" dropdown, with that expiry). Picking a duration only changes the selection; the left half resolves. Defaults to Once. +**Approval variant**: Left = runs the selected duration, labelled "Allow once" / "Allow for 24 hours" / "Allow always". Right dropdown = duration menu: **Once** (plain allow), then an "Allow & remember" section with **For 1 hour / 24 hours / 7 days / 30 days** and **Always** (remember at the scope chosen in the action bar's "Remember as" dropdown, with that expiry). Picking a duration only changes the selection; the left half resolves. Defaults to Once. The org's default remember expiry (`default_remember_ttl_secs`) is tagged "· default" in the menu, and gets its own "For …" entry when it is not one of the listed durations. Every remember choice sends `ttl` explicitly, so "Always" is `forever`, not an omission. ### Notifications Dropdown @@ -657,9 +657,6 @@ User-level preferences. Changes take effect immediately. - **Format**: Relative ("2m ago") / Absolute local / Absolute UTC - **Timezone**: auto-detected from browser, overridable -**Approvals:** -- **Default approval TTL**: pre-filled expiry when clicking "Allow & Remember" (1h, 24h, 7d, 30d, never) - **Notifications:** - **Notification preferences**: how to receive approval requests and secret requests — email, webhook URL, or dashboard-only @@ -859,7 +856,6 @@ Features Allow user-created templates [✓] Users can create personal service templates Allow user-created services [✓] Users can create personal service instances Show API Explorer [✓] API Explorer visible in nav for all users -Default approval TTL [24h ▾] Pre-filled expiry for "Allow & Remember" ``` Each with a toggle or dropdown. Settings configured via environment variables are shown as read-only with an "env" badge. @@ -900,8 +896,9 @@ One card, admin-only, holding the org policies applied to an agent **at creation - **New agents can set up their own services** (`default_agent_self_setup`, default **on**) — a first-level agent is seeded with the four `overslash:*_own` self-setup rules. The help text names what it does *not* include (the `_share`/`_publish` half) and warns that an agent owned by an org admin inherits admin reach through its owner's groups regardless. - **Deferred execution by default for new agents** (`default_deferred_execution`, default off). - **Upstream call timeouts** — default and maximum, blank to inherit the deployment value. +- **Default "Allow & Remember" expiry** (`default_remember_ttl_secs`, default **7 days**; Never / 1h / 24h / 7d / 30d). Unlike the rows above it is not a creation-time default: it applies to every remember resolve from the moment it changes. It is what an API or MCP resolve that omits `ttl` gets, and the approval's Allow menu tags that duration "· default" (the split button itself still starts on Once). Sits below the timeouts behind a divider so the card's "applied at creation time" description doesn't read as covering it. -All three patch the same `PATCH /v1/orgs/{id}/execution-settings` endpoint, one key at a time. +All of these patch the same `PATCH /v1/orgs/{id}/execution-settings` endpoint, one key at a time. **Backfill control.** Because the self-setup default applies at agent-creation time only, an org that predates it has agents the policy never reached. A row under that toggle says how many (`agents_missing_self_setup`, carried on the same settings response) and offers a button labelled *Grant to N agents*, which `POST`s to `/v1/orgs/{id}/agent-self-setup/backfill` behind a confirm dialog. It is **disabled while the toggle is off**, with a tooltip saying to turn the default on first — granting against a declined policy would hand out what the org just opted out of. When nothing is pending the row reads "Every first-level agent in this org already has these rules" and offers no button. There is no bulk undo; the confirm dialog says so, and revocation stays per agent in the Permission Rules table. diff --git a/crates/overslash-api/src/routes/approvals/resolve.rs b/crates/overslash-api/src/routes/approvals/resolve.rs index 4e789ea2a..5791c2dee 100644 --- a/crates/overslash-api/src/routes/approvals/resolve.rs +++ b/crates/overslash-api/src/routes/approvals/resolve.rs @@ -234,18 +234,23 @@ pub(super) async fn resolve_approval( let mut parsed_expires_at: Option = None; let mut remember_keys_to_store: Option> = None; if remember { - if let Some(t) = req.ttl.as_deref() { - let dur = overslash_core::types::duration::parse_ttl(t) - .ok_or_else(|| AppError::BadRequest(format!("invalid ttl: {t}")))?; - if dur.as_secs() > 365 * 86400 { - return Err(AppError::BadRequest("ttl must not exceed 365 days".into())); + // An explicit `ttl` wins (`"forever"` = no expiry); an omitted one + // falls back to the org's default, which starts at 7 days. + let ttl_secs: Option = match req.ttl.as_deref() { + Some(t) => overslash_core::types::duration::parse_rule_ttl(t) + .map_err(|e| AppError::BadRequest(e.to_string()))? + .map(|d| d.as_secs() as i64), + None => { + overslash_db::repos::org::get_default_remember_ttl_secs(state.db(&ext), auth.org_id) + .await? + .ok_or_else(|| AppError::NotFound("org not found".into()))? + .map(i64::from) } - let secs: i64 = dur - .as_secs() - .try_into() - .map_err(|_| AppError::BadRequest("ttl value too large".into()))?; + }; + if let Some(secs) = ttl_secs { + // `parse_rule_ttl` and the column CHECK both cap this at 365 days. parsed_expires_at = - time::OffsetDateTime::now_utc().checked_add(time::Duration::new(secs, 0)); + time::OffsetDateTime::now_utc().checked_add(time::Duration::seconds(secs)); } let approval = &approval_pre; diff --git a/crates/overslash-api/src/routes/mcp/elicitation.rs b/crates/overslash-api/src/routes/mcp/elicitation.rs index da0d88608..f0c8f9829 100644 --- a/crates/overslash-api/src/routes/mcp/elicitation.rs +++ b/crates/overslash-api/src/routes/mcp/elicitation.rs @@ -100,6 +100,7 @@ pub(super) fn supports_form_elicitation(capabilities: &Value) -> bool { pub(super) fn sse_elicitation_response( state: AppState, ext: axum::http::Extensions, + org_id: Uuid, rpc_id: Value, elicit_id: String, action_summary: String, @@ -115,6 +116,7 @@ pub(super) fn sse_elicitation_response( let stream = elicitation_event_stream( state, ext, + org_id, rpc_id, elicit_id, elicit_request, @@ -138,6 +140,7 @@ enum StreamStep { fn elicitation_event_stream( state: AppState, ext: axum::http::Extensions, + org_id: Uuid, rpc_id: Value, elicit_id: String, elicit_request: Value, @@ -171,11 +174,12 @@ fn elicitation_event_stream( let outcome = mcp_session::await_completion(&state, &ext, &elicit_id).await; let (frame, next) = match outcome { mcp_session::ElicitOutcome::FollowUp(next_id) if follow_ups_left > 0 => { + let default_ttl = org_remember_ttl_secs(&state, &ext, org_id).await; let frame = json!({ "jsonrpc": "2.0", "id": next_id, "method": "elicitation/create", - "params": remember_params(&pending_outcome), + "params": remember_params(&pending_outcome, default_ttl), }); let next = StreamStep::Poll { elicit_id: next_id, @@ -299,7 +303,10 @@ pub(super) fn elicitation_params(action_summary: &str, pending_outcome: &Value) /// them against the approval, so nothing here is trusted. The narrowest tier /// is the default. With no tiers the scope field is left out and the /// approval's own keys are remembered. -pub(super) fn remember_params(pending_outcome: &Value) -> Value { +/// +/// `default_ttl_secs` is the org's default expiry (`None` = forever), from +/// [`org_remember_ttl_secs`]; it pre-selects the duration field. +pub(super) fn remember_params(pending_outcome: &Value, default_ttl_secs: Option) -> Value { let action_summary = pending_outcome .get("action_description") .and_then(Value::as_str) @@ -347,19 +354,28 @@ pub(super) fn remember_params(pending_outcome: &Value) -> Value { }), ); } + let mut ttl_options: Vec = TTL_PRESETS + .iter() + .map(|(value, _, title)| json!({ "const": value, "title": title })) + .collect(); + let default_ttl = match TTL_PRESETS.iter().find(|(_, s, _)| *s == default_ttl_secs) { + Some((value, _, _)) => value.to_string(), + // An org default set through the API to an off-menu duration still + // has to be selectable, or the dialog would pre-select nothing. + None => { + let secs = default_ttl_secs.unwrap_or_default(); + let value = format!("{secs}s"); + ttl_options.push(json!({ "const": value, "title": humanize_secs(secs) })); + value + } + }; properties.insert( "ttl".into(), json!({ "type": "string", "title": "For how long", - "oneOf": [ - { "const": "forever", "title": "Forever" }, - { "const": "1h", "title": "1 hour" }, - { "const": "24h", "title": "24 hours" }, - { "const": "7d", "title": "7 days" }, - { "const": "30d", "title": "30 days" } - ], - "default": "forever" + "oneOf": ttl_options, + "default": default_ttl }), ); @@ -373,6 +389,48 @@ pub(super) fn remember_params(pending_outcome: &Value) -> Value { }) } +/// The remember dialog's duration menu: `(ttl value, seconds, title)`, in the +/// order the dashboard's `ExpiryControl` lists them. `None` seconds = forever. +const TTL_PRESETS: [(&str, Option, &str); 5] = [ + ("forever", None, "Forever"), + ("1h", Some(3_600), "1 hour"), + ("24h", Some(86_400), "24 hours"), + ("7d", Some(604_800), "7 days"), + ("30d", Some(2_592_000), "30 days"), +]; + +/// What the remember dialog pre-selects when the org's setting cannot be +/// read — the column's own default. +const FALLBACK_REMEMBER_TTL_SECS: i32 = 604_800; + +/// The org's default "Allow & remember" expiry (`None` = forever). A failed +/// lookup falls back to 7 days rather than failing the dialog: it is only a +/// pre-selection, and `/resolve` re-reads the setting for an omitted `ttl`. +pub(super) async fn org_remember_ttl_secs( + state: &AppState, + ext: &axum::http::Extensions, + org_id: Uuid, +) -> Option { + match overslash_db::repos::org::get_default_remember_ttl_secs(state.db(ext), org_id).await { + Ok(Some(secs)) => secs, + Ok(None) => Some(FALLBACK_REMEMBER_TTL_SECS), + Err(e) => { + tracing::warn!(%org_id, "read default_remember_ttl_secs failed: {e}"); + Some(FALLBACK_REMEMBER_TTL_SECS) + } + } +} + +/// "2 days", "90 minutes" — the largest whole unit an off-menu duration fits. +fn humanize_secs(secs: i32) -> String { + let (n, unit) = [(86_400, "day"), (3_600, "hour"), (60, "minute")] + .into_iter() + .find(|(size, _)| secs % size == 0) + .map(|(size, unit)| (secs / size, unit)) + .unwrap_or((secs, "second")); + format!("{n} {unit}{}", if n == 1 { "" } else { "s" }) +} + /// The descriptive render-form fields the envelope carries, so either dialog /// can show *what* is being approved (the labeled disclosure summary + risk /// class) and the scope ladder, mirroring the dashboard review card. All read @@ -493,7 +551,7 @@ mod tests { { "keys": ["gmail:send:to=bob@x"], "description": "Emails to bob@x" }, { "keys": ["gmail:send:*", "gmail:read:*"], "description": "Any send or read" }, ]); - let p = remember_params(&env); + let p = remember_params(&env, Some(604_800)); let props = &p["requestedSchema"]["properties"]; let scope = &props["scope"]; @@ -505,13 +563,39 @@ mod tests { vec!["gmail:send:*", "gmail:read:*"], "a multi-key tier must round-trip through its string value" ); - assert_eq!(props["ttl"]["default"], "forever"); + assert_eq!(props["ttl"]["default"], "7d"); assert_eq!(p["message"], "Remember permission to: send an email"); } + /// The org default pre-selects the duration; "forever" stays a choice, + /// and an off-menu default is appended so it can still be the default. + #[test] + fn remember_dialog_defaults_to_the_org_ttl() { + let ttl = |secs| { + remember_params(&envelope(), secs)["requestedSchema"]["properties"]["ttl"].clone() + }; + + assert_eq!(ttl(None)["default"], "forever"); + assert_eq!(ttl(Some(3_600))["default"], "1h"); + assert_eq!(ttl(Some(3_600))["oneOf"].as_array().unwrap().len(), 5); + + let custom = ttl(Some(172_800)); + assert_eq!(custom["default"], "172800s"); + let last = custom["oneOf"].as_array().unwrap().last().unwrap().clone(); + assert_eq!(last, json!({ "const": "172800s", "title": "2 days" })); + } + + #[test] + fn humanize_secs_picks_the_largest_whole_unit() { + assert_eq!(humanize_secs(86_400), "1 day"); + assert_eq!(humanize_secs(5_400), "90 minutes"); + assert_eq!(humanize_secs(7_200), "2 hours"); + assert_eq!(humanize_secs(61), "61 seconds"); + } + #[test] fn remember_dialog_without_tiers_asks_only_for_duration() { - let p = remember_params(&envelope()); + let p = remember_params(&envelope(), Some(604_800)); let props = p["requestedSchema"]["properties"].as_object().unwrap(); assert_eq!(props.keys().collect::>(), vec!["ttl"]); } diff --git a/crates/overslash-api/src/routes/mcp/initialize.rs b/crates/overslash-api/src/routes/mcp/initialize.rs index da8ed1bcb..1d32afac9 100644 --- a/crates/overslash-api/src/routes/mcp/initialize.rs +++ b/crates/overslash-api/src/routes/mcp/initialize.rs @@ -170,7 +170,7 @@ pub(super) async fn tools_list_result( }, "ttl": { "type": "string", - "description": "Duration the remembered rule stays live (e.g. `24h`, `30d`). Only meaningful with `allow_remember`." + "description": "Duration the remembered rule stays live (e.g. `24h`, `30d`), or `forever` for no expiry. Omitted, the org's default applies (7 days unless an admin changed it). Only meaningful with `allow_remember`." } }, "required": ["approval_id", "resolution"], diff --git a/crates/overslash-api/src/routes/mcp/modern.rs b/crates/overslash-api/src/routes/mcp/modern.rs index 2d5315f5c..981f62765 100644 --- a/crates/overslash-api/src/routes/mcp/modern.rs +++ b/crates/overslash-api/src/routes/mcp/modern.rs @@ -31,7 +31,7 @@ use base64::Engine as _; use sha2::{Digest, Sha256}; use super::elicitation::{ - elicit_result, elicitation_eligible, elicitation_params, remember_params, + elicit_result, elicitation_eligible, elicitation_params, org_remember_ttl_secs, remember_params, }; use super::initialize::{server_info, server_instructions, tools_list_result}; use super::tools_call::{Reply, pending_approval_result, tools_call}; @@ -430,6 +430,7 @@ pub(super) async fn begin_elicitation( Ok(request_state) => Reply::Result(input_required( STEP_DECISION, pending_outcome, + None, &request_state, )), Err(e) => { @@ -534,9 +535,15 @@ pub(super) async fn continue_elicitation( .and_then(|r| r.get(claims.step.as_str())) .filter(|a| a.is_object()); let Some(answer) = answer else { + let ttl = if claims.step == STEP_REMEMBER { + org_remember_ttl_secs(state, ext, auth.org_id).await + } else { + None + }; return Reply::Result(input_required( &claims.step, &claims.envelope, + ttl, request_state, )); }; @@ -569,7 +576,13 @@ pub(super) async fn continue_elicitation( &claims.envelope, ) { Ok(next_state) => { - Reply::Result(input_required(STEP_REMEMBER, &claims.envelope, &next_state)) + let ttl = org_remember_ttl_secs(state, ext, auth.org_id).await; + Reply::Result(input_required( + STEP_REMEMBER, + &claims.envelope, + ttl, + &next_state, + )) } Err(e) => { tracing::error!(next_id, "mint mcp request state failed: {e}"); @@ -629,9 +642,16 @@ async fn answer_stale( } /// An `input_required` result asking one dialog, keyed by its step. -fn input_required(step: &str, pending_outcome: &Value, request_state: &str) -> Value { +/// `remember_ttl_secs` (the org default, `None` = forever) only shapes the +/// remember step. +fn input_required( + step: &str, + pending_outcome: &Value, + remember_ttl_secs: Option, + request_state: &str, +) -> Value { let mut params = if step == STEP_REMEMBER { - remember_params(pending_outcome) + remember_params(pending_outcome, remember_ttl_secs) } else { let action_summary = pending_outcome .get("action_description") @@ -909,7 +929,7 @@ mod tests { #[test] fn input_required_names_the_dialog_by_its_step_and_marks_it_form() { let env = json!({ "action_description": "send an email", "suggested_tiers": [] }); - let r = input_required(STEP_DECISION, &env, "state"); + let r = input_required(STEP_DECISION, &env, None, "state"); assert_eq!(r["resultType"], "input_required"); assert_eq!(r["requestState"], "state"); let ask = &r["inputRequests"][STEP_DECISION]; @@ -920,10 +940,10 @@ mod tests { "Allow this agent to: send an email?" ); - let r = input_required(STEP_REMEMBER, &env, "state"); - assert!( - r["inputRequests"][STEP_REMEMBER]["params"]["requestedSchema"]["properties"]["ttl"] - .is_object() + let r = input_required(STEP_REMEMBER, &env, Some(604_800), "state"); + assert_eq!( + r["inputRequests"][STEP_REMEMBER]["params"]["requestedSchema"]["properties"]["ttl"]["default"], + "7d" ); } diff --git a/crates/overslash-api/src/routes/mcp/tools_call.rs b/crates/overslash-api/src/routes/mcp/tools_call.rs index e2bd27b8d..5b81943a9 100644 --- a/crates/overslash-api/src/routes/mcp/tools_call.rs +++ b/crates/overslash-api/src/routes/mcp/tools_call.rs @@ -386,6 +386,7 @@ async fn tools_call_overslash_call( Reply::Stream(sse_elicitation_response( state.clone(), ext.clone(), + auth.org_id, req.id.clone(), elicit_id, action_summary, diff --git a/crates/overslash-api/src/routes/orgs/mod.rs b/crates/overslash-api/src/routes/orgs/mod.rs index 8a0808bcc..d629ab020 100644 --- a/crates/overslash-api/src/routes/orgs/mod.rs +++ b/crates/overslash-api/src/routes/orgs/mod.rs @@ -101,6 +101,10 @@ const MIN_RETENTION_DAYS: i32 = 1; const MIN_CALL_TIMEOUT_MS: i32 = 1_000; const MAX_CALL_TIMEOUT_MS: i32 = 600_000; const MAX_RETENTION_DAYS: i32 = 60; +/// Bounds for `default_remember_ttl_secs`: one minute to the 365-day cap any +/// remembered rule carries (mirrors the `orgs_default_remember_ttl_bounds` CHECK). +const MIN_REMEMBER_TTL_SECS: i32 = 60; +const MAX_REMEMBER_TTL_SECS: i32 = 365 * 86_400; #[derive(Serialize)] pub(super) struct OrgResponse { diff --git a/crates/overslash-api/src/routes/orgs/settings.rs b/crates/overslash-api/src/routes/orgs/settings.rs index 4f5139a22..5ffe763fa 100644 --- a/crates/overslash-api/src/routes/orgs/settings.rs +++ b/crates/overslash-api/src/routes/orgs/settings.rs @@ -329,6 +329,11 @@ pub(super) struct ExecutionSettingsResponse { /// inherits `CALL_TIMEOUT_MAX_MS`. A caller asking for more is /// rejected; a template or org *default* above it is clamped. max_call_timeout_ms: Option, + /// Expiry, in seconds, of a rule written by "Allow & Remember" when the + /// resolver passes no `ttl`. Tagged as the default in the dashboard's + /// Allow menu, and preselected in the MCP remember dialog. `null` = + /// forever. Default: 7 days. + default_remember_ttl_secs: Option, } /// Partial patch: an absent key leaves the setting alone. @@ -348,6 +353,8 @@ pub(super) struct PatchExecutionSettingsRequest { call_timeout_ms: Option>, #[serde(default, deserialize_with = "double_option")] max_call_timeout_ms: Option>, + #[serde(default, deserialize_with = "double_option")] + default_remember_ttl_secs: Option>, } /// Distinguish an absent key from an explicit `null`. @@ -380,6 +387,9 @@ pub(super) async fn get_execution_settings( let timeouts = overslash_db::repos::org::get_call_settings(state.db(&ext), id) .await? .ok_or_else(|| AppError::NotFound("org not found".into()))?; + let remember_ttl = overslash_db::repos::org::get_default_remember_ttl_secs(state.db(&ext), id) + .await? + .ok_or_else(|| AppError::NotFound("org not found".into()))?; let missing = overslash_db::repos::org_bootstrap::count_agents_missing_self_setup(state.db(&ext), id) .await?; @@ -389,6 +399,7 @@ pub(super) async fn get_execution_settings( agents_missing_self_setup: Some(missing), call_timeout_ms: timeouts.call_timeout_ms, max_call_timeout_ms: timeouts.max_call_timeout_ms, + default_remember_ttl_secs: remember_ttl, })) } @@ -421,6 +432,15 @@ pub(super) async fn patch_execution_settings( } } + if let Some(Some(secs)) = req.default_remember_ttl_secs + && !(MIN_REMEMBER_TTL_SECS..=MAX_REMEMBER_TTL_SECS).contains(&secs) + { + return Err(AppError::BadRequest(format!( + "default_remember_ttl_secs must be between {MIN_REMEMBER_TTL_SECS} and \ + {MAX_REMEMBER_TTL_SECS} seconds (1 minute to 365 days), or null for forever" + ))); + } + // The cross-field rule spans a value this patch may not mention, so the // read, the check and the write happen together under a row lock inside // the repo — validating here against a separate read would let two @@ -434,12 +454,13 @@ pub(super) async fn patch_execution_settings( default_agent_self_setup: req.default_agent_self_setup, call_timeout_ms: req.call_timeout_ms, max_call_timeout_ms: req.max_call_timeout_ms, + default_remember_ttl_secs: req.default_remember_ttl_secs, }, ) .await?; use overslash_db::repos::org::ExecutionSettingsUpdate; - let (next_deferred, next_self_setup, next_call, next_max) = match outcome { + let (next_deferred, next_self_setup, next_call, next_max, next_remember_ttl) = match outcome { ExecutionSettingsUpdate::NotFound => { return Err(AppError::NotFound("org not found".into())); } @@ -458,11 +479,13 @@ pub(super) async fn patch_execution_settings( default_agent_self_setup, call_timeout_ms, max_call_timeout_ms, + default_remember_ttl_secs, } => ( default_deferred_execution, default_agent_self_setup, call_timeout_ms, max_call_timeout_ms, + default_remember_ttl_secs, ), }; @@ -478,6 +501,7 @@ pub(super) async fn patch_execution_settings( "default_agent_self_setup": next_self_setup, "call_timeout_ms": next_call, "max_call_timeout_ms": next_max, + "default_remember_ttl_secs": next_remember_ttl, }), description: None, ip_address: ip.0.as_deref(), @@ -498,6 +522,7 @@ pub(super) async fn patch_execution_settings( agents_missing_self_setup: missing, call_timeout_ms: next_call, max_call_timeout_ms: next_max, + default_remember_ttl_secs: next_remember_ttl, })) } diff --git a/crates/overslash-api/src/routes/permissions.rs b/crates/overslash-api/src/routes/permissions.rs index 65f4a7939..f06265603 100644 --- a/crates/overslash-api/src/routes/permissions.rs +++ b/crates/overslash-api/src/routes/permissions.rs @@ -273,19 +273,13 @@ struct UpdatePermissionRequest { /// clears the expiry. fn ttl_to_expires_at(ttl: Option<&str>) -> Result> { let Some(t) = ttl else { return Ok(None) }; - if t == "forever" { + let Some(dur) = overslash_core::types::duration::parse_rule_ttl(t) + .map_err(|e| AppError::BadRequest(e.to_string()))? + else { return Ok(None); - } - let dur = overslash_core::types::duration::parse_ttl(t) - .ok_or_else(|| AppError::BadRequest(format!("invalid ttl: {t}")))?; - if dur.as_secs() > 365 * 86400 { - return Err(AppError::BadRequest("ttl must not exceed 365 days".into())); - } - let secs: i64 = dur - .as_secs() - .try_into() - .map_err(|_| AppError::BadRequest("ttl value too large".into()))?; - Ok(OffsetDateTime::now_utc().checked_add(time::Duration::new(secs, 0))) + }; + // Capped at 365 days by `parse_rule_ttl`, so this cannot overflow. + Ok(OffsetDateTime::now_utc().checked_add(time::Duration::seconds(dur.as_secs() as i64))) } async fn update_permission( diff --git a/crates/overslash-api/src/services/mcp_session.rs b/crates/overslash-api/src/services/mcp_session.rs index 35507fd04..cfb7f6810 100644 --- a/crates/overslash-api/src/services/mcp_session.rs +++ b/crates/overslash-api/src/services/mcp_session.rs @@ -466,6 +466,10 @@ pub async fn complete_from_elicitation( /// the narrowest safe default. No trust is placed in the value: `/resolve` /// refuses any key that is neither a suggested tier nor covers a requested /// key, so a forged `scope` fails there. +/// +/// `ttl` is forwarded verbatim, `"forever"` included: an omitted `ttl` means +/// the org's default expiry, so dropping "forever" would quietly turn it into +/// 7 days. fn remember_resolve_body(content: &Value) -> Value { let mut body = json!({ "resolution": "allow_remember" }); let keys: Vec = content @@ -478,9 +482,7 @@ fn remember_resolve_body(content: &Value) -> Value { if !keys.is_empty() { body["remember_keys"] = json!(keys); } - if let Some(ttl) = content.get("ttl").and_then(Value::as_str) - && ttl != "forever" - { + if let Some(ttl) = content.get("ttl").and_then(Value::as_str) { body["ttl"] = json!(ttl); } body @@ -535,12 +537,8 @@ mod tests { /// approval's own keys" — the resolver's default when the field is /// absent, and a 400 if we forwarded `[]` instead. #[test] - fn remember_body_omits_keys_it_cannot_use_and_forever_ttl() { - for content in [ - json!({ "ttl": "forever" }), - json!({ "scope": "not json" }), - json!({ "scope": "[]" }), - ] { + fn remember_body_omits_keys_it_cannot_use() { + for content in [json!({ "scope": "not json" }), json!({ "scope": "[]" })] { assert_eq!( remember_resolve_body(&content), json!({ "resolution": "allow_remember" }), @@ -548,4 +546,14 @@ mod tests { ); } } + + /// "forever" is an explicit choice; omitting it would hand the resolver + /// the org default instead. + #[test] + fn remember_body_forwards_forever() { + assert_eq!( + remember_resolve_body(&json!({ "ttl": "forever" })), + json!({ "resolution": "allow_remember", "ttl": "forever" }) + ); + } } diff --git a/crates/overslash-api/tests/approvals_remember_keys.rs b/crates/overslash-api/tests/approvals_remember_keys.rs index 9db538fe6..98c48ec24 100644 --- a/crates/overslash-api/tests/approvals_remember_keys.rs +++ b/crates/overslash-api/tests/approvals_remember_keys.rs @@ -7,6 +7,10 @@ //! the dashboard's "Custom… (advanced)" field produces, while still rejecting //! an unrelated grant. Rules are written once per distinct key — a multi-key //! approval whose tier collapses onto one key must not write it N times. +//! +//! The rule's expiry: an explicit `ttl` wins (`"forever"` = none), and an +//! omitted one takes the org's `default_remember_ttl_secs` — 7 days unless an +//! admin changed it. use crate::common; @@ -257,3 +261,199 @@ async fn repeated_remember_keys_write_one_rule() { "one distinct key must write exactly one rule" ); } + +/// Resolve "Allow & Remember" on the approval's own keys with the given +/// `ttl` (omitted when `None`), replay it, and return the written rule's +/// `expires_at` as seconds from now (`None` = forever). +async fn remember_and_read_expiry( + base: &str, + org_key: &str, + agent_key: &str, + approval_id: Uuid, + agent_id: Uuid, + ttl: Option<&str>, +) -> Option { + let mut body = json!({"resolution": "allow_remember"}); + if let Some(ttl) = ttl { + body["ttl"] = json!(ttl); + } + let resp = reqwest::Client::new() + .post(format!("{base}/v1/approvals/{approval_id}/resolve")) + .header("Authorization", format!("Bearer {org_key}")) + .json(&body) + .send() + .await + .unwrap(); + assert_eq!( + resp.status(), + 200, + "{}", + resp.text().await.unwrap_or_default() + ); + + let resp = reqwest::Client::new() + .post(format!("{base}/v1/approvals/{approval_id}/call")) + .header("Authorization", format!("Bearer {agent_key}")) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + + let rows: Value = reqwest::Client::new() + .get(format!("{base}/v1/permissions?identity_id={agent_id}")) + .header("Authorization", format!("Bearer {org_key}")) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + let rule = rows + .as_array() + .unwrap() + .iter() + .find(|r| { + !r["action_pattern"] + .as_str() + .unwrap() + .starts_with("overslash:") + }) + .expect("the remembered rule"); + rule["expires_at"].as_str().map(|at| { + let at = time::OffsetDateTime::parse(at, &time::format_description::well_known::Rfc3339) + .unwrap(); + (at - time::OffsetDateTime::now_utc()).whole_seconds() + }) +} + +async fn patch_remember_ttl(base: &str, key: &str, org_id: Uuid, secs: Value) -> reqwest::Response { + reqwest::Client::new() + .patch(format!("{base}/v1/orgs/{org_id}/execution-settings")) + .header("Authorization", format!("Bearer {key}")) + .json(&json!({"default_remember_ttl_secs": secs})) + .send() + .await + .unwrap() +} + +/// Within a minute of `expected` seconds from now — the resolve and the read +/// are a few round trips apart. +fn assert_expires_in(got: Option, expected: i64) { + let got = got.expect("rule should expire"); + assert!( + (expected - 60..=expected).contains(&got), + "expected ~{expected}s to expiry, got {got}s" + ); +} + +#[tokio::test] +async fn omitted_ttl_takes_the_seven_day_org_default() { + let (pool, fx) = common::test_pool_bootstrapped().await; + let (base, org_key, agent_key, approval_id, agent_id, _mock_addr) = + pending_approval(pool, &fx).await; + + let settings: Value = reqwest::Client::new() + .get(format!("{base}/v1/orgs/{}/execution-settings", fx.org_id)) + .header("Authorization", format!("Bearer {org_key}")) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + assert_eq!(settings["default_remember_ttl_secs"], 604_800); + + let expiry = + remember_and_read_expiry(&base, &org_key, &agent_key, approval_id, agent_id, None).await; + assert_expires_in(expiry, 604_800); +} + +/// "forever" used to be spelled by leaving `ttl` out. Now that an omission +/// means the org default, it has to be accepted as an explicit value. +#[tokio::test] +async fn explicit_forever_writes_a_permanent_rule() { + let (pool, fx) = common::test_pool_bootstrapped().await; + let (base, org_key, agent_key, approval_id, agent_id, _mock_addr) = + pending_approval(pool, &fx).await; + + let expiry = remember_and_read_expiry( + &base, + &org_key, + &agent_key, + approval_id, + agent_id, + Some("forever"), + ) + .await; + assert_eq!(expiry, None); +} + +#[tokio::test] +async fn explicit_ttl_beats_the_org_default() { + let (pool, fx) = common::test_pool_bootstrapped().await; + let (base, org_key, agent_key, approval_id, agent_id, _mock_addr) = + pending_approval(pool, &fx).await; + + let expiry = remember_and_read_expiry( + &base, + &org_key, + &agent_key, + approval_id, + agent_id, + Some("1h"), + ) + .await; + assert_expires_in(expiry, 3_600); +} + +#[tokio::test] +async fn a_changed_org_default_applies_to_omitted_ttl() { + let (pool, fx) = common::test_pool_bootstrapped().await; + let (base, org_key, agent_key, approval_id, agent_id, _mock_addr) = + pending_approval(pool, &fx).await; + + let resp = patch_remember_ttl(&base, &org_key, fx.org_id, json!(86_400)).await; + assert_eq!(resp.status(), 200); + assert_eq!( + resp.json::().await.unwrap()["default_remember_ttl_secs"], + 86_400 + ); + + let expiry = + remember_and_read_expiry(&base, &org_key, &agent_key, approval_id, agent_id, None).await; + assert_expires_in(expiry, 86_400); +} + +#[tokio::test] +async fn a_forever_org_default_writes_permanent_rules() { + let (pool, fx) = common::test_pool_bootstrapped().await; + let (base, org_key, agent_key, approval_id, agent_id, _mock_addr) = + pending_approval(pool, &fx).await; + + let resp = patch_remember_ttl(&base, &org_key, fx.org_id, Value::Null).await; + assert_eq!(resp.status(), 200); + assert!(resp.json::().await.unwrap()["default_remember_ttl_secs"].is_null()); + + let expiry = + remember_and_read_expiry(&base, &org_key, &agent_key, approval_id, agent_id, None).await; + assert_eq!(expiry, None); +} + +#[tokio::test] +async fn org_default_ttl_is_bounded_and_admin_only() { + let (pool, fx) = common::test_pool_bootstrapped().await; + let (base, org_key, agent_key, _approval_id, _agent_id, _mock_addr) = + pending_approval(pool, &fx).await; + + for secs in [0, 59, 31_536_001] { + let resp = patch_remember_ttl(&base, &org_key, fx.org_id, json!(secs)).await; + assert_eq!(resp.status(), 400, "{secs}s must be rejected"); + } + + let resp = patch_remember_ttl(&base, &agent_key, fx.org_id, json!(3_600)).await; + assert!( + resp.status().is_client_error(), + "a non-admin must not change the org default: {}", + resp.status() + ); +} diff --git a/crates/overslash-api/tests/mcp_elicitation.rs b/crates/overslash-api/tests/mcp_elicitation.rs index 4a994ede9..336f50df9 100644 --- a/crates/overslash-api/tests/mcp_elicitation.rs +++ b/crates/overslash-api/tests/mcp_elicitation.rs @@ -955,7 +955,8 @@ async fn remember_dialog_saves_the_picked_tier_with_its_ttl() { } /// No scope picked (a client that skips defaults) remembers the approval's -/// own keys — the narrowest rule — with no expiry. +/// own keys — the narrowest rule — and no duration picked takes the org's +/// default expiry, 7 days out of the box. #[tokio::test] async fn remember_dialog_without_scope_saves_the_exact_keys() { let fx = bootstrap_mcp(true).await; @@ -980,7 +981,14 @@ async fn remember_dialog_without_scope_saves_the_exact_keys() { ); let rules = new_rules(&fx, &before).await; assert!(!rules.is_empty(), "expected a remembered rule"); - assert!(rules.iter().all(|(_, e)| e.is_none()), "{rules:?}"); + let in_a_week = time::OffsetDateTime::now_utc() + time::Duration::days(7); + for (pattern, expires) in rules { + let expires = expires.expect("org default should set an expiry"); + assert!( + (expires - in_a_week).abs() < time::Duration::minutes(2), + "{pattern} expires at {expires}, want ≈ {in_a_week}" + ); + } } /// Declining the follow-up is backing out of the details after saying diff --git a/crates/overslash-core/src/types/duration.rs b/crates/overslash-core/src/types/duration.rs index d1bf9e9f2..285025676 100644 --- a/crates/overslash-core/src/types/duration.rs +++ b/crates/overslash-core/src/types/duration.rs @@ -42,10 +42,50 @@ pub fn parse_ttl(s: &str) -> Option { } } +/// Longest expiry a remembered permission rule may carry. +pub const MAX_RULE_TTL_SECS: u64 = 365 * 86400; + +#[derive(Debug, PartialEq, Eq, thiserror::Error)] +pub enum RuleTtlError { + #[error("invalid ttl: {0}")] + Invalid(String), + #[error("ttl must not exceed 365 days")] + TooLong, +} + +/// Parse the `ttl` a permission rule is written with: `"forever"` is no +/// expiry (`None`), anything else is a [`parse_ttl`] duration capped at +/// [`MAX_RULE_TTL_SECS`]. +pub fn parse_rule_ttl(s: &str) -> Result, RuleTtlError> { + if s == "forever" { + return Ok(None); + } + let dur = parse_ttl(s).ok_or_else(|| RuleTtlError::Invalid(s.to_string()))?; + if dur.as_secs() > MAX_RULE_TTL_SECS { + return Err(RuleTtlError::TooLong); + } + Ok(Some(dur)) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn test_parse_rule_ttl() { + assert_eq!(parse_rule_ttl("forever"), Ok(None)); + assert_eq!(parse_rule_ttl("7d"), Ok(Some(Duration::from_secs(604800)))); + assert_eq!( + parse_rule_ttl("365d"), + Ok(Some(Duration::from_secs(MAX_RULE_TTL_SECS))) + ); + assert_eq!(parse_rule_ttl("366d"), Err(RuleTtlError::TooLong)); + assert_eq!( + parse_rule_ttl("never"), + Err(RuleTtlError::Invalid("never".into())) + ); + } + #[test] fn test_parse_ttl() { assert_eq!(parse_ttl("24h"), Some(Duration::from_secs(86400))); diff --git a/crates/overslash-db/migrations/139_org_default_remember_ttl.down.sql b/crates/overslash-db/migrations/139_org_default_remember_ttl.down.sql new file mode 100644 index 000000000..bfe62da3a --- /dev/null +++ b/crates/overslash-db/migrations/139_org_default_remember_ttl.down.sql @@ -0,0 +1,3 @@ +ALTER TABLE orgs + DROP CONSTRAINT orgs_default_remember_ttl_bounds, + DROP COLUMN default_remember_ttl_secs; diff --git a/crates/overslash-db/migrations/139_org_default_remember_ttl.up.sql b/crates/overslash-db/migrations/139_org_default_remember_ttl.up.sql new file mode 100644 index 000000000..37a200574 --- /dev/null +++ b/crates/overslash-db/migrations/139_org_default_remember_ttl.up.sql @@ -0,0 +1,25 @@ +-- Per-org default expiry for "Allow & Remember" rules. +-- +-- Before this, a remembered rule lived forever unless the approver changed the +-- expiry picker, and an API/MCP resolve that omitted `ttl` minted a permanent +-- rule. Forever is the riskiest choice to make by default: a standing grant +-- nobody revisits. Orgs now pick the default, and it starts at 7 days. +-- +-- NULL = forever (no expiry), the same Option::None the rule's own +-- `expires_at` uses. The column DEFAULT backfills every existing org to 7 days +-- on purpose — the safer default should reach orgs that never opted in. +-- +-- The 60s..365d bounds mirror the resolver's own ttl cap, so a default can +-- never mint a rule an explicit `ttl` could not. + +ALTER TABLE orgs + ADD COLUMN default_remember_ttl_secs INTEGER DEFAULT 604800; + +ALTER TABLE orgs + ADD CONSTRAINT orgs_default_remember_ttl_bounds CHECK ( + default_remember_ttl_secs IS NULL + OR default_remember_ttl_secs BETWEEN 60 AND 31536000 + ); + +COMMENT ON COLUMN orgs.default_remember_ttl_secs IS + 'Default expiry in seconds for rules written by Allow & Remember when the resolver does not pass a ttl. NULL = forever.'; diff --git a/crates/overslash-db/src/repos/org.rs b/crates/overslash-db/src/repos/org.rs index 2c83eab08..62e478f75 100644 --- a/crates/overslash-db/src/repos/org.rs +++ b/crates/overslash-db/src/repos/org.rs @@ -346,6 +346,22 @@ pub async fn get_default_deferred_execution( Ok(row.map(|r| r.default_deferred_execution)) } +/// Read the org's default expiry, in seconds, for rules written by +/// "Allow & Remember" when the resolver passes no `ttl`. The inner `None` +/// means forever; the outer `None` means the org doesn't exist. +pub async fn get_default_remember_ttl_secs( + pool: &PgPool, + id: Uuid, +) -> Result>, sqlx::Error> { + let row = sqlx::query!( + "SELECT default_remember_ttl_secs FROM orgs WHERE id = $1", + id, + ) + .fetch_optional(pool) + .await?; + Ok(row.map(|r| r.default_remember_ttl_secs)) +} + /// Update the `default_deferred_execution` setting for an org. pub async fn set_default_deferred_execution( pool: &PgPool, @@ -454,6 +470,7 @@ pub enum ExecutionSettingsUpdate { default_agent_self_setup: bool, call_timeout_ms: Option, max_call_timeout_ms: Option, + default_remember_ttl_secs: Option, }, } @@ -484,6 +501,8 @@ pub struct ExecutionSettingsPatch { pub call_timeout_ms: Option>, /// Same three-valued shape as `call_timeout_ms`. pub max_call_timeout_ms: Option>, + /// Same three-valued shape; `Some(None)` means "remember forever". + pub default_remember_ttl_secs: Option>, } pub async fn update_execution_settings( @@ -496,16 +515,22 @@ pub async fn update_execution_settings( default_agent_self_setup, call_timeout_ms, max_call_timeout_ms, + default_remember_ttl_secs, } = patch; let (set_call_timeout, call_timeout_ms) = (call_timeout_ms.is_some(), call_timeout_ms.flatten()); let (set_max_call_timeout, max_call_timeout_ms) = (max_call_timeout_ms.is_some(), max_call_timeout_ms.flatten()); + let (set_remember_ttl, default_remember_ttl_secs) = ( + default_remember_ttl_secs.is_some(), + default_remember_ttl_secs.flatten(), + ); let mut tx = pool.begin().await?; let Some(current) = sqlx::query!( - "SELECT default_deferred_execution, default_agent_self_setup, call_timeout_ms, max_call_timeout_ms + "SELECT default_deferred_execution, default_agent_self_setup, call_timeout_ms, max_call_timeout_ms, + default_remember_ttl_secs FROM orgs WHERE id = $1 FOR UPDATE", id, ) @@ -525,6 +550,11 @@ pub async fn update_execution_settings( } else { current.max_call_timeout_ms }; + let next_remember_ttl = if set_remember_ttl { + default_remember_ttl_secs + } else { + current.default_remember_ttl_secs + }; let next_deferred = default_deferred_execution.unwrap_or(current.default_deferred_execution); let next_self_setup = default_agent_self_setup.unwrap_or(current.default_agent_self_setup); @@ -544,6 +574,7 @@ pub async fn update_execution_settings( default_agent_self_setup = $3, call_timeout_ms = $4, max_call_timeout_ms = $5, + default_remember_ttl_secs = $6, updated_at = now() WHERE id = $1", id, @@ -551,6 +582,7 @@ pub async fn update_execution_settings( next_self_setup, next_call, next_max, + next_remember_ttl, ) .execute(&mut *tx) .await?; @@ -561,6 +593,7 @@ pub async fn update_execution_settings( default_agent_self_setup: next_self_setup, call_timeout_ms: next_call, max_call_timeout_ms: next_max, + default_remember_ttl_secs: next_remember_ttl, }) } diff --git a/dashboard/scripts/screenshot-remember-ttl-default.mjs b/dashboard/scripts/screenshot-remember-ttl-default.mjs new file mode 100644 index 000000000..81e98f209 --- /dev/null +++ b/dashboard/scripts/screenshot-remember-ttl-default.mjs @@ -0,0 +1,72 @@ +// Real-stack screenshots for the org default "Allow & Remember" expiry: the +// Org Settings row that sets it, and an approval whose Allow menu tags that +// duration as the default (the split button itself still starts on Once). +// +// Prereq: `make e2e-up`. Output: dashboard/screenshots/remember-ttl-*.png. + +import { + login, + makeSnapper, + seedApproval, + setDefaultRememberTtl +} from '../tests/scenarios/index.mjs'; + +const session = await login('admin'); +const snap = await makeSnapper(session); +const CARD = 'Agent defaults'; + +try { + // ── 1. The org setting, at its out-of-the-box 7 days ──────────────── + await setDefaultRememberTtl(session, 604800); + const { page, ctx } = await snap.navigateAndSnap('remember-ttl-org-page', '/org/agent-defaults', { + viewport: { width: 1400, height: 1100 }, + fullPage: false, + waitFor: async (p) => { + const card = p.locator('section.card', { hasText: CARD }).first(); + await card.waitFor({ timeout: 20_000 }); + await p.getByLabel('Default Allow & Remember expiry').scrollIntoViewIfNeeded(); + await p.waitForTimeout(400); + } + }); + await page + .locator('section.card', { hasText: CARD }) + .first() + .screenshot({ path: 'screenshots/remember-ttl-org-card.png' }); + console.log('[scenarios] wrote screenshots/remember-ttl-org-card.png'); + + // Change it through the UI so the round trip is real. + await page.getByLabel('Default Allow & Remember expiry').selectOption('30d'); + await page.waitForTimeout(800); + await ctx.close(); + + // ── 2. An approval: the Allow menu tags the org default (30 days) ─── + const approval = await seedApproval(session); + const { page: aPage, ctx: aCtx } = await snap.navigateAndSnap( + 'remember-ttl-approval', + `/approvals/${approval.id}`, + { + viewport: { width: 1400, height: 1000 }, + fullPage: false, + waitFor: async (p) => { + await p.getByRole('button', { name: 'More allow options' }).waitFor({ timeout: 20_000 }); + await p.waitForTimeout(300); + } + } + ); + await aPage.getByRole('button', { name: 'More allow options' }).click(); + await aPage + .getByRole('menuitemradio', { name: /For 30 days · default/ }) + .waitFor({ timeout: 10_000 }); + await aPage.waitForTimeout(400); // let the menu's fade-in finish + // The menu drops below the action bar, so clip a region covering both. + const bar = await aPage.locator('.aq-actionbar').first().boundingBox(); + await aPage.screenshot({ + path: 'screenshots/remember-ttl-approval-actionbar.png', + clip: { x: bar.x, y: bar.y, width: bar.width, height: bar.height + 300 } + }); + console.log('[scenarios] wrote screenshots/remember-ttl-approval-actionbar.png'); + await aCtx.close(); +} finally { + await setDefaultRememberTtl(session, 604800).catch(() => {}); + await snap.close(); +} diff --git a/dashboard/src/lib/approvals/format.ts b/dashboard/src/lib/approvals/format.ts index 7b819616f..76b3a445c 100644 --- a/dashboard/src/lib/approvals/format.ts +++ b/dashboard/src/lib/approvals/format.ts @@ -22,14 +22,46 @@ export function pickApiError(e: unknown, fallback = 'Something went wrong.'): st return e instanceof Error ? e.message : fallback; } +// `secs` is the same duration as the org's `default_remember_ttl_secs` +// (null = forever), so the org default maps onto a menu entry. export const TTL_OPTIONS = [ - { value: 'forever', label: 'Never' }, - { value: '1h', label: '1 hour' }, - { value: '24h', label: '24 hours' }, - { value: '7d', label: '7 days' }, - { value: '30d', label: '30 days' } + { value: 'forever', label: 'Never', secs: null }, + { value: '1h', label: '1 hour', secs: 3600 }, + { value: '24h', label: '24 hours', secs: 86400 }, + { value: '7d', label: '7 days', secs: 604800 }, + { value: '30d', label: '30 days', secs: 2592000 } ] as const; +/** The org default assumed before the real one has loaded (the column default). */ +export const DEFAULT_REMEMBER_TTL = '7d'; + +/** + * The org's `default_remember_ttl_secs` as a `ttl` the resolver accepts: the + * matching menu value, or `"{n}s"` for a duration set through the API that is + * not on the menu. + */ +export function ttlFromSecs(secs: number | null): string { + return TTL_OPTIONS.find((o) => o.secs === secs)?.value ?? `${secs}s`; +} + +/** Display label for a `ttl` value — the menu label, or "2 days" off-menu. */ +export function ttlLabel(value: string): string { + const preset = TTL_OPTIONS.find((o) => o.value === value); + if (preset) return preset.label; + const secs = Number(value.match(/^(\d+)s$/)?.[1]); + if (!secs) return value; + const [size, unit] = ( + [ + [86400, 'day'], + [3600, 'hour'], + [60, 'minute'], + [1, 'second'] + ] as const + ).find(([size]) => secs % size === 0)!; + const n = secs / size; + return `${n} ${unit}${n === 1 ? '' : 's'}`; +} + const HUMANIZED: Record = { github: 'GitHub', gitlab: 'GitLab', diff --git a/dashboard/src/lib/components/ApprovalDetail.svelte b/dashboard/src/lib/components/ApprovalDetail.svelte index cd2dd031f..34cd20c76 100644 --- a/dashboard/src/lib/components/ApprovalDetail.svelte +++ b/dashboard/src/lib/components/ApprovalDetail.svelte @@ -24,8 +24,11 @@ utf8ByteLength, splitDisclosed, rememberKeys, - resolutionToast + resolutionToast, + ttlFromSecs, + DEFAULT_REMEMBER_TTL } from '#lib/approvals/format.ts'; + import type { ExecutionSettings } from '#lib/types.ts'; import { onMount } from 'svelte'; let { @@ -65,6 +68,10 @@ let customKey = $state(''); // 'once' or a TTL_OPTIONS value — what the Allow split button runs. let dur = $state('once'); + // The org's default expiry for "Allow & Remember" — what the gateway applies + // to a resolve that omits `ttl`. Tagged in the Allow menu, not preselected: + // the split button still defaults to Once. + let defaultTtl = $state(DEFAULT_REMEMBER_TTL); let formError = $state(null); const submitting = $derived(ctrl.submitting); const error = $derived(formError ?? ctrl.error); @@ -96,6 +103,19 @@ formError = null; }); + const orgId = $derived((page.data as { user?: { org_id?: string } })?.user?.org_id ?? null); + onMount(async () => { + if (!orgId) return; + try { + const settings = await session.get( + `/v1/orgs/${orgId}/execution-settings` + ); + defaultTtl = ttlFromSecs(settings.default_remember_ttl_secs); + } catch { + // Keep the 7-day fallback; it only labels the menu. + } + }); + const riskMeta = { low: { glyph: '○', label: 'Low risk' }, med: { glyph: '◐', label: 'Medium risk' }, @@ -175,7 +195,8 @@ return; } body.remember_keys = keys; - if (dur !== 'forever') body.ttl = dur; + // Always explicit: an omitted ttl means the org default, not "Always". + body.ttl = dur; } const updated = await ctrl.resolve(body); if (!updated) return; @@ -235,7 +256,7 @@ - + {#if willRunInBackground} diff --git a/dashboard/src/lib/components/approval/AllowSplit.svelte b/dashboard/src/lib/components/approval/AllowSplit.svelte index e052f6aee..aaf886168 100644 --- a/dashboard/src/lib/components/approval/AllowSplit.svelte +++ b/dashboard/src/lib/components/approval/AllowSplit.svelte @@ -5,30 +5,44 @@ // duration as the rule's expiry — so once-vs-remember and the expiry are one // decision instead of two buttons plus a clock menu. - import { TTL_OPTIONS } from '#lib/approvals/format.ts'; + import { TTL_OPTIONS, ttlLabel } from '#lib/approvals/format.ts'; let { dur = $bindable('once'), + defaultTtl, disabled = false, canRemember = true, onallow }: { dur?: string; + /** The org's default remember expiry, tagged "default" in the menu. */ + defaultTtl?: string; disabled?: boolean; canRemember?: boolean; onallow: (dur: string) => void; } = $props(); - // `forever` sits last as "Always"; the rest read "For ". - const DURS = [ + // `forever` sits last as "Always"; the rest read "For ". An org + // default set through the API to an off-menu duration gets its own entry, + // so the menu can always name it. + const DURS = $derived([ { value: 'once', label: 'Once', btn: 'once' }, ...TTL_OPTIONS.filter((o) => o.value !== 'forever').map((o) => ({ value: o.value as string, label: `For ${o.label}`, btn: `for ${o.label}` })), + ...(defaultTtl && !TTL_OPTIONS.some((o) => o.value === defaultTtl) + ? [ + { + value: defaultTtl, + label: `For ${ttlLabel(defaultTtl)}`, + btn: `for ${ttlLabel(defaultTtl)}` + } + ] + : []), { value: 'forever', label: 'Always', btn: 'always' } - ]; + ]); let open = $state(false); let root: HTMLDivElement | undefined = $state(); @@ -86,7 +100,10 @@ open = false; }} > - {d.label} + {d.label}{#if d.value === defaultTtl}{' '}· default{/if} {#if d.value === dur}✓{/if} {/each} @@ -95,6 +112,10 @@