From 72eecab0ec529e7d970e4c9ce9d2ff36efcfb25f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81ngel=20Manuel=20Mart=C3=ADn?= Date: Tue, 27 Oct 2026 21:40:09 +0100 Subject: [PATCH 01/10] feat(infra): make Cloud Run services public via invoker_iam_disabled (#840) The overspiral.com GCP org enforces domain-restricted sharing (iam.allowedPolicyMemberDomains), which rejects new allUsers invoker bindings. Disable the invoker IAM check on the api, overfwd and shortener services instead, and forget the old bindings with `removed` blocks so the apply never leaves a service IAM-gated mid-flight. Co-authored-by: Claude Opus 5.5 --- infra/modules/cloud-run-overfwd/main.tf | 23 +++++++++++++---------- infra/modules/cloud-run-shortener/main.tf | 22 ++++++++++++---------- infra/modules/cloud-run/main.tf | 22 ++++++++++++---------- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/infra/modules/cloud-run-overfwd/main.tf b/infra/modules/cloud-run-overfwd/main.tf index 25071a95..44cc1832 100644 --- a/infra/modules/cloud-run-overfwd/main.tf +++ b/infra/modules/cloud-run-overfwd/main.tf @@ -129,10 +129,11 @@ resource "google_secret_manager_secret_iam_member" "env_secrets" { } resource "google_cloud_run_v2_service" "overfwd" { - name = "${var.base_prefix}-overfwd" - location = var.region - project = var.project_id - ingress = "INGRESS_TRAFFIC_ALL" + name = "${var.base_prefix}-overfwd" + location = var.region + project = var.project_id + ingress = "INGRESS_TRAFFIC_ALL" + invoker_iam_disabled = true template { service_account = var.service_account_email @@ -231,12 +232,14 @@ resource "google_cloud_run_v2_service" "overfwd" { # (OVERFWD_REQUIRE_API_KEY above), and orgs running their own overfwd reach a # different deployment entirely. IAM-gating this would not help — the API calls # it as an ordinary upstream HTTP service, not as an authenticated GCP client. -resource "google_cloud_run_v2_service_iam_member" "public" { - project = var.project_id - location = var.region - name = google_cloud_run_v2_service.overfwd.name - role = "roles/run.invoker" - member = "allUsers" +# +# Public via `invoker_iam_disabled` on the service rather than an `allUsers` +# invoker binding, which the overspiral.com org's domain-restricted sharing +# policy (iam.allowedPolicyMemberDomains) rejects. `removed` forgets the old +# binding without deleting it, so the service never flips to IAM-gated +# mid-apply; drop the leftover binding by hand afterwards. +removed { + from = google_cloud_run_v2_service_iam_member.public } resource "google_cloud_run_domain_mapping" "domain" { diff --git a/infra/modules/cloud-run-shortener/main.tf b/infra/modules/cloud-run-shortener/main.tf index 7548b733..8900a48a 100644 --- a/infra/modules/cloud-run-shortener/main.tf +++ b/infra/modules/cloud-run-shortener/main.tf @@ -158,10 +158,11 @@ resource "google_secret_manager_secret_iam_member" "env_secrets" { } resource "google_cloud_run_v2_service" "shortener" { - name = "${var.base_prefix}-shortener" - location = var.region - project = var.project_id - ingress = "INGRESS_TRAFFIC_ALL" + name = "${var.base_prefix}-shortener" + location = var.region + project = var.project_id + ingress = "INGRESS_TRAFFIC_ALL" + invoker_iam_disabled = true template { service_account = var.service_account_email @@ -252,12 +253,13 @@ resource "google_cloud_run_v2_service" "shortener" { } } -resource "google_cloud_run_v2_service_iam_member" "public" { - project = var.project_id - location = var.region - name = google_cloud_run_v2_service.shortener.name - role = "roles/run.invoker" - member = "allUsers" +# Public via `invoker_iam_disabled` on the service rather than an `allUsers` +# invoker binding, which the overspiral.com org's domain-restricted sharing +# policy (iam.allowedPolicyMemberDomains) rejects. `removed` forgets the old +# binding without deleting it, so the service never flips to IAM-gated +# mid-apply; drop the leftover binding by hand afterwards. +removed { + from = google_cloud_run_v2_service_iam_member.public } resource "google_cloud_run_domain_mapping" "domain" { diff --git a/infra/modules/cloud-run/main.tf b/infra/modules/cloud-run/main.tf index 68ed209b..7bdef3aa 100644 --- a/infra/modules/cloud-run/main.tf +++ b/infra/modules/cloud-run/main.tf @@ -579,10 +579,11 @@ resource "google_secret_manager_secret_version" "otel_config" { # secret rule (modules/secret-manager). resource "google_cloud_run_v2_service" "api" { - name = "${var.base_prefix}-api" - location = var.region - project = var.project_id - ingress = var.ingress + name = "${var.base_prefix}-api" + location = var.region + project = var.project_id + ingress = var.ingress + invoker_iam_disabled = true template { service_account = var.service_account_email @@ -761,12 +762,13 @@ resource "google_cloud_run_v2_service" "api" { } } -resource "google_cloud_run_v2_service_iam_member" "public" { - project = var.project_id - location = var.region - name = google_cloud_run_v2_service.api.name - role = "roles/run.invoker" - member = "allUsers" +# Public via `invoker_iam_disabled` on the service rather than an `allUsers` +# invoker binding, which the overspiral.com org's domain-restricted sharing +# policy (iam.allowedPolicyMemberDomains) rejects. `removed` forgets the old +# binding without deleting it, so the service never flips to IAM-gated +# mid-apply; drop the leftover binding by hand afterwards. +removed { + from = google_cloud_run_v2_service_iam_member.public } resource "google_cloud_run_domain_mapping" "domain" { From 5fab14f4ccb97813f4ca63f8e8d073bf2132b5b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81ngel=20Manuel=20Mart=C3=ADn?= Date: Tue, 27 Oct 2026 22:52:48 +0100 Subject: [PATCH 02/10] fix(deps): bump block-buffer 0.12.1, except 0.10.4 and deepmerge (#842) OSV started flagging GHSA-qwgh-2vcv-g2f7 (block-buffer) and GHSA-gcqx-w48m-w2h4 (deepmerge), turning the daily Dependency audit and every PR's OSV scan red. block-buffer 0.12 moves to the fixed 0.12.1. 0.10.4 (pinned by digest 0.10) and deepmerge (dev-only) have no fixed release; both are excepted for 90 days with the reasoning in dependency-vulnerability-policy.md. Co-authored-by: Factory Co-authored-by: Claude Opus 5.5 --- Cargo.lock | 8 ++++---- .../casa/dependency-vulnerability-policy.md | 2 ++ osv-scanner.toml | 12 ++++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d0dc00cf..7bc72aad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -505,9 +505,9 @@ dependencies = [ [[package]] name = "block-buffer" -version = "0.12.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cdd35008169921d80bc60d3d0ab416eecb028c4cd653352907921d95084790be" +checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa" dependencies = [ "hybrid-array", ] @@ -630,7 +630,7 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e8cf2a2c93cd704877c0858356ed03480ff301ee950b43f1cbe4573b088bfa6c" dependencies = [ - "block-buffer 0.12.0", + "block-buffer 0.12.1", "crypto-common 0.2.2", "inout", ] @@ -1147,7 +1147,7 @@ version = "0.11.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4850db49bf08e663084f7fb5c87d202ef91a3907271aff24a94eb97ff039153c" dependencies = [ - "block-buffer 0.12.0", + "block-buffer 0.12.1", "const-oid 0.10.2", "crypto-common 0.2.2", "ctutils", diff --git a/docs/compliance/casa/dependency-vulnerability-policy.md b/docs/compliance/casa/dependency-vulnerability-policy.md index 234a9488..a7481d4e 100644 --- a/docs/compliance/casa/dependency-vulnerability-policy.md +++ b/docs/compliance/casa/dependency-vulnerability-policy.md @@ -154,6 +154,8 @@ reviewer: | [RUSTSEC-2026-0387](https://rustsec.org/advisories/RUSTSEC-2026-0387) | `crypto-bigint 0.5.5` | Vulnerability (operations meant to be constant-time could branch on secret values: subtraction, division, Montgomery multiplication, limb equality), unscored | The only fixed release is 0.7.6, a semver-major. `elliptic-curve` 0.13 pins `^0.5` and no `jsonwebtoken` release has moved off it, so there is no compatible bump. `crypto-bigint` arrives only via `jsonwebtoken`'s `rust_crypto` backend (`p256`/`p384` → `elliptic-curve`), which compiles every algorithm in. Overslash uses **no EC algorithm**: HS256 in `crates/overslash-api/src/services/jwt.rs`, RS256 in `services/google_directory.rs`. No EC key is ever constructed, so the vulnerable arithmetic is linked but unreachable. Re-check whether `jsonwebtoken` has moved to `crypto-bigint` 0.7, or switch it to the `aws_lc_rs` backend | 2026-12-22 | | [RUSTSEC-2024-0436](https://rustsec.org/advisories/RUSTSEC-2024-0436) | `paste 1.0.15` | Unmaintained (not a vulnerability) | No maintained release to move to. It is a proc-macro, so it runs at build time and contributes no runtime code. Pulled in by `tokenizers` and `rav1e`, both through `fastembed`. Re-check whether `tokenizers` has moved to a maintained fork | 2026-12-22 | | [RUSTSEC-2024-0449](https://rustsec.org/advisories/RUSTSEC-2024-0449) | `unsafe-libyaml 0.2.11` | Unmaintained (not a vulnerability) | No maintained release to move to. Unlike `paste` it is runtime code: `serde_yaml` (deprecated) uses it in `overslash-core` to parse service templates, including org-authored ones. The fix is moving off `serde_yaml` (e.g. to `serde-saphyr`), a code change rather than a bump. Re-check whether that migration has landed | 2026-12-22 | +| [GHSA-qwgh-2vcv-g2f7](https://osv.dev/GHSA-qwgh-2vcv-g2f7) | `block-buffer 0.10.4` | Vulnerability (a caught panic leaves the buffer cursor invalid, then out-of-bounds read/write), CVSS 6.3 | GHSA-only, so OSV flags it and cargo-deny does not. The 0.12 line is bumped to the fixed 0.12.1. 0.10.x has no fix, and `digest` 0.10 users (`sha2`, `hmac` 0.12, `rsa`, `ecdsa`) pin it. Exploitation needs the compression callback to panic *and* the panic to be caught while the buffer is still used. The callbacks are the hash crates' own compression functions, which do not panic, and the only `catch_unwind` calls in the codebase wrap config parsing. Re-check whether those crates have moved to `digest` 0.11 | 2027-01-25 | +| [GHSA-gcqx-w48m-w2h4](https://osv.dev/GHSA-gcqx-w48m-w2h4) | `deepmerge 4.3.1` (npm, dev) | Vulnerability (prototype poisoning in `mergeObject`), CVSS 5.3 | No fixed release. Dev dependency only, via `@sveltejs/vite-plugin-svelte` merging the build config we write. No external input reaches it and it is not in any shipped bundle | 2027-01-25 | ## Evidence for an assessor diff --git a/osv-scanner.toml b/osv-scanner.toml index 2b503f64..3a31c3c5 100644 --- a/osv-scanner.toml +++ b/osv-scanner.toml @@ -30,3 +30,15 @@ reason = "paste is unmaintained, not vulnerable; build-time proc-macro via faste id = "RUSTSEC-2024-0449" ignoreUntil = 2026-12-22 reason = "unsafe-libyaml is unmaintained, not vulnerable; template parsing via serde_yaml. No maintained release; fix is migrating off serde_yaml. See deny.toml." + +# GHSA-only (no RustSec ID, so cargo-deny does not see it and deny.toml has no +# entry). The 0.12 line is bumped to the fixed 0.12.1; only 0.10.4 is excepted. +[[IgnoredVulns]] +id = "GHSA-qwgh-2vcv-g2f7" +ignoreUntil = 2027-01-25 +reason = "block-buffer 0.10.4 (via digest 0.10: sha2, hmac 0.12, rsa, ecdsa): needs a panicking compress callback whose panic is caught while the buffer stays in use. The callbacks are the hash crates' own compression functions, which do not panic, and no catch_unwind wraps a hasher. No 0.10.x fix; 0.12 is bumped to 0.12.1." + +[[IgnoredVulns]] +id = "GHSA-gcqx-w48m-w2h4" +ignoreUntil = 2027-01-25 +reason = "deepmerge 4.3.1 prototype poisoning: dev-only, via @sveltejs/vite-plugin-svelte merging our own build config. No attacker-supplied input reaches it and it ships in no bundle. No fixed release." From 050d5ecfed9a4529763bfeb51ee08c40881f64ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81ngel=20Manuel=20Mart=C3=ADn?= Date: Tue, 27 Oct 2026 23:22:23 +0100 Subject: [PATCH 03/10] feat(infra): remove default-SA Editor and set project org policies (#841) * feat(infra): remove default-SA Editor and set project org policies Google's IAM Recommender flagged roles/editor on the default Compute Engine SA (0 permissions used in 90 days) and on the cloudservices agent (12 used, all the VPC connector's instance group). Both were project- creation defaults that nothing in Terraform owned. modules/project-guardrails removes both with google_project_iam_member_remove, granting the agent roles/compute.editor first where the VPC connector exists. It also sets project-level org policies now that both projects sit in the overspiral.com org: the default-SA pair, the audit-exemption ban, public access prevention, and the Cloud SQL public-IP pair (enforced in prod, explicitly off on dev). An import block adopts dev's hand-set key-creation opt-out. Not applied: docs/runbooks/project-guardrails.md has the operator steps. Dev's default-SA binding was already removed by hand. Co-Authored-By: Claude Opus 5.5 * fix(infra): make the Cloud Run removed blocks' destroy = false explicit A bare removed block already forgets the old allUsers invoker binding without deleting it; OpenTofu warns that the intent is implicit. Same behaviour, no warning. Co-Authored-By: Claude Opus 5.5 * docs(runbooks): ADC quota project for the org-policy apply Co-Authored-By: Claude Opus 5.5 * ci(infra): validate with OpenTofu 1.13 so removed blocks may carry lifecycle Co-Authored-By: Claude Opus 5.5 * fix(infra): bill provider API quota to the managed project Org Policy rejects user-ADC calls without a quota project, so `make tofu-plan` failed reading project-guardrails policies. Set user_project_override + billing_project on both providers and enable Cloud Resource Manager in the API list, which the provider's own reads now count against. Co-Authored-By: Claude Opus 5.5 * docs(runbooks): quota project now comes from the provider config Co-Authored-By: Claude Opus 5.5 * docs(casa): record the applied org-policy baseline and guardrails Both projects are in the overspiral.com org, which enforces a baseline at org level; project-guardrails is applied to dev and prod. Replace the pending-apply notes with the measured effective policy per constraint, note the removed allUsers invoker bindings, and narrow the open items to run.allowedIngress and the Recommender capture. Co-Authored-By: Claude Opus 5.5 * docs(casa): track run.allowedIngress as its own evidence row Co-Authored-By: Claude Opus 5.5 * docs(todo): split run.allowedIngress out of the org-policy baseline item Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Factory Co-authored-by: Claude Opus 5.5 --- .github/workflows/ci.yml | 2 +- TODO.md | 5 +- docs/compliance/casa/evidence-index.md | 2 + docs/compliance/casa/gap-assessment.md | 4 +- docs/compliance/casa/gcp-posture.md | 64 ++++++++ docs/runbooks/project-guardrails.md | 177 ++++++++++++++++++++++ infra/README.md | 1 + infra/env/dev.tfvars | 4 + infra/main.tf | 26 ++++ infra/modules/cloud-run-overfwd/main.tf | 4 + infra/modules/cloud-run-shortener/main.tf | 4 + infra/modules/cloud-run/main.tf | 4 + infra/modules/project-guardrails/main.tf | 173 +++++++++++++++++++++ infra/providers.tf | 12 ++ infra/variables.tf | 6 + 15 files changed, 483 insertions(+), 5 deletions(-) create mode 100644 docs/runbooks/project-guardrails.md create mode 100644 infra/modules/project-guardrails/main.tf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c6786568..45c717bd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -454,7 +454,7 @@ jobs: - name: Install OpenTofu uses: opentofu/setup-opentofu@a1320f892987e89d278cc92dc5adc984fb93aca4 # v2.0.2 with: - tofu_version: "1.8" + tofu_version: "1.13" tofu_wrapper: false - name: Format check diff --git a/TODO.md b/TODO.md index 385af39d..60035b06 100644 --- a/TODO.md +++ b/TODO.md @@ -112,7 +112,7 @@ are gaps today. **P2 — raised by Google's own Recommender.** Measured on **both** `overslash` and `overslash-dev`; see [gcp-posture.md](docs/compliance/casa/gcp-posture.md). Remediating these closes them in Google's console, which is itself the evidence artifact. - [x] **P2 `REQUIRE_SSL`** — **production** `overslash-prod-db` runs `sslMode=ALLOW_UNENCRYPTED_AND_ENCRYPTED`, behind a **HIGH**-severity insight. `ssl_mode = "ENCRYPTED_ONLY"` is now set in `infra/modules/cloud-sql/main.tf`, so both environments get it. Every client was checked first: the API (and its in-process migrations) and the metrics-exporter job use the `/cloudsql` Auth Proxy volume, BigQuery federation uses a `cloud_sql` connection, humans use `bin/db-shell.sh` — all TLS, all admitted under any mode. **Not applied** — the Recommender closes it only after a human runs [cloud-sql-hardening.md](docs/runbooks/cloud-sql-hardening.md). (CASA 4.1.1) -- [ ] **P2 unused `roles/editor`** — on production, **two** GCP-default service accounts hold Editor: the default Compute Engine SA (**0 permissions used in 90 days** → remove) and the cloudservices agent (12 used → downgrade to `roles/compute.editor`). Both invisible to Terraform. Add `constraints/iam.automaticIamGrantsForDefaultServiceAccounts` so they do not come back. (CASA 3.1.1) +- [x] **P2 unused `roles/editor`** — on production, **two** GCP-default service accounts held Editor: the default Compute Engine SA (**0 permissions used in 90 days** → remove) and the cloudservices agent (12 used, all the VPC connector's instance group → `roles/compute.editor`). Dev had only the first. Checked first that nothing depends on the default Compute SA: every Cloud Run service and job, Cloud Build trigger and Scheduler job names its own account; there are no instances, keys or `actAs` grants; the only build ever run as it was the manual submit of 2026-04-29. **Applied in prod 2026-10-27** (re-measured: only `compute.editor` on the cloudservices agent remains): `infra/modules/project-guardrails/` removes both with `google_project_iam_member_remove`, so a re-added binding is drift that the next apply removes, and grants `compute.editor` first where `use_private_vpc`. **Dev's binding was removed by hand on 2026-10-27** (`overslash-dev-api` `/health` still 200). Both projects now sit in the overspiral.com org, whose inherited policies already enforce `iam.automaticIamGrantsForDefaultServiceAccounts`. The module pins it at project level and adds `iam.managed.preventPrivilegedBasicRolesForDefaultServiceAccounts`, so the grants cannot come back by creation or by a later grant. Verify per [project-guardrails.md](docs/runbooks/project-guardrails.md); the Recommender closing both P2s is the remaining evidence. (CASA 3.1.1) - [x] **P3/P4 Cloud SQL policies + auditing** — no instance or user password policy, and no database auditing. Added `log_connections`, `log_disconnections`, `cloudsql.enable_pgaudit`, `pgaudit.log = ddl,role` and `cloudsql.pgaudit_mask_literals` (so the API's boot-time `ALTER ROLE bi … PASSWORD` is not logged in clear) as database flags; the one-off `CREATE EXTENSION pgaudit` is in the runbook. pgAudit output lands in Data Access logs, so it is only readable once the audit-config item below is done. Password policies **declined**, with the reasoning in [gcp-posture.md](docs/compliance/casa/gcp-posture.md#password-policy-decision). Not applied yet — [cloud-sql-hardening.md](docs/runbooks/cloud-sql-hardening.md). (CASA 1.1.1, 6.7.1) - [x] **Audit logging is entirely off** — the project IAM policy returns `auditConfigs: NONE`, so Secret Manager, Cloud SQL and Cloud Run Data Access logs are not written. `google_project_iam_audit_config` for all three, a sink to a locked bucket and a log-based alert on `AccessSecretVersion` are in `infra/modules/audit-logging/`. Live until applied — same operator step as the 6.7.1 item above. (CASA 6.7.1) - [x] **`_Default` log bucket is 30 days and unlocked** — no export sink exists. Everything an assessor wants to see is in the bucket that is neither retained nor tamper-evident. Every Cloud Audit Log is now also sunk to `overslash--audit` (400 days, locked in prod). `_Default` itself stays 30 days on purpose: it holds application logs, and the app's own audit trail is the Postgres `audit_log`. Same operator step. (CASA 6.7.1) @@ -122,7 +122,8 @@ are gaps today. - [x] **Memorystore has neither AUTH nor transit encryption** — `overslash-prod-valkey` confirmed live: `authEnabled` unset, `transitEncryptionMode: DISABLED`, `BASIC` tier. **In code, pending apply:** `infra/modules/memorystore/main.tf` sets `auth_enabled = true` and `transit_encryption_mode = "SERVER_AUTHENTICATION"`, and publishes the AUTH string and server CA to Secret Manager (`overslash--valkey-auth`, `-valkey-server-ca`), which reach the API (`REDIS_PASSWORD`, `REDIS_CA_CERT`) and oversla-sh (`VALKEY_*`) by `secret_key_ref`. Both clients connect `rediss://` and verify the server against only that CA (`services/valkey.rs`, `oversla-sh/src/storage.rs`). **Operator step still open:** TLS forces a replacement of the instance (downtime, empty keyspace, oversla.sh links lost unless exported) — pre-flight, apply and verify per [memorystore-auth-tls.md](docs/runbooks/memorystore-auth-tls.md). (CASA 6.7.1) - [ ] **Production has 0 uptime checks and no `[P0] API Down` alert** — 8 alert policies are deployed, which is exactly 12 declared minus 3 disabled in tfvars minus the API-down policy `count`-gated on `api_domain != ""`. The only deployed P0 is `API High 5xx Rate`, which cannot fire when the service returns nothing at all. - [x] **No SSL policy and no Cloud Armor on the production LB** — both `gcloud compute ssl-policies list` and `security-policies list` are empty. TLS 1.2/1.3-only is currently true but unpinned and uncitable. `infra/modules/api-lb/main.tf` now declares `overslash-prod-api-ssl-policy` (TLS 1.2 minimum, `RESTRICTED`) on the HTTPS proxy and `overslash-prod-api-armor` on the backend service — default allow, OWASP CRS rule sets in preview (logged, never blocked) until tuned. **Not applied** — in code, pending a human run of [lb-hardening.md](docs/runbooks/lb-hardening.md), which also lists the evidence commands. (CASA 4.1.1) -- [ ] Set the org-policy baseline that would make the above guardrails rather than conventions: `sql.restrictPublicIp`, `iam.disableServiceAccountKeyCreation`, `run.allowedIngress`, `storage.publicAccessPrevention`. None is currently set. +- [x] Set the org-policy baseline that would make the above guardrails rather than conventions. **Applied 2026-10-27** in both projects. The overspiral.com org now enforces a baseline at org level (measured table in [gcp-posture.md](docs/compliance/casa/gcp-posture.md#supply-chain-and-organization-policy)), and `infra/modules/project-guardrails/` ([project-guardrails.md](docs/runbooks/project-guardrails.md)) pins the Overslash-specific ones per project: `sql.managed.restrictPublicIp` + `restrictAuthorizedNetworks` (enforced in prod, explicitly off on dev's public-IP database), `storage.publicAccessPrevention` and `iam.disableAuditLoggingExemption` (both projects). `iam.managed.disableServiceAccountKeyCreation` is inherited from the org, and dev's hand-set opt-out for the Directory-sync key is adopted into Terraform. Org-wide policies belong to the org repo, not here. +- [ ] **`run.allowedIngress`** — unset (allow all) on both projects, measured 2026-10-27. It applies project-wide, so it can only be set to `internal-and-cloud-load-balancing` once overfwd and the shortener stop running with `INGRESS_TRAFFIC_ALL` (prod API is already LB-only). (CASA 6.2.1) - [x] `Keyring::test()` is compiled only under `cfg(test)` or overslash-core's `test-support` feature, which only `overslash-api`'s `[dev-dependencies]` enable — the hardcoded `[0xAB; 32]` key is no longer in the production library. - [x] **Trusted-proxy configuration for `X-Forwarded-For`**: `services/client_ip.rs`. `ClientIp` now takes the rightmost *untrusted* address instead of the leftmost header value, trusting hops by position (`OVERSLASH_TRUSTED_PROXY_HOPS`), by range (`OVERSLASH_TRUSTED_PROXIES`), and the Vercel rewrite hop by a shared secret its Routing Middleware stamps (`OVERSLASH_TRUSTED_PROXY_SECRET`, `dashboard/middleware.ts`). With nothing configured, the socket peer is the client. Parsed at boot, and a malformed value refuses to start. The two handlers that parsed XFF inline (`/oauth/register`, MCP upstream `initiate`) go through the extractor too. prod/dev values and the enable/rotate/verify steps are in `infra/README.md`. Tests in `tests/client_ip.rs`. **Operator step still open:** set the secret in GSM and Vercel per env, then flip `enable_trusted_proxy_secret`. - [x] `Cache-Control: no-store` on `secrets/reveal` and other sensitive authenticated responses — the default on every API response that does not set its own (`/icons/*` keeps `public, max-age=86400`; streamed and deferred-download passthrough keeps the upstream's). diff --git a/docs/compliance/casa/evidence-index.md b/docs/compliance/casa/evidence-index.md index 9d69757e..0906d11e 100644 --- a/docs/compliance/casa/evidence-index.md +++ b/docs/compliance/casa/evidence-index.md @@ -93,4 +93,6 @@ what the product actually renders. |-----|----------|--------| | 6.1.1 | Dependency scan output: the *Dependency audit* jobs (`cargo-deny`, `npm audit`, OSV). Take a recent scheduled run against `master`, which shows the two justified ignores filtered with their reasons | `have`: GitHub Actions, `.github/workflows/deps-audit.yml` | | 3.1.1 | `gcloud storage buckets describe` for `overslash_cloudbuild` and `overslash-dev_cloudbuild`: UBLA `true`, public-access prevention `enforced`, the 30-day delete rule, an empty `acl`, and no objects | `have` — measured 2026-10-23 on both projects, every field as listed. Re-capture with the verify step of [cloud-build-bucket.md](../../runbooks/cloud-build-bucket.md) | +| 3.1.1 | `gcloud projects get-iam-policy` on both projects showing no `roles/editor` on the default Compute SA or the cloudservices agent; `gcloud org-policies describe` for the project-level constraints; the IAM Recommender's two `roles/editor` P2s closed | `need` — applied in both projects 2026-10-27 and measured: no `roles/editor` on either default account (prod's cloudservices agent holds `roles/compute.editor`), project and org-level constraints effective per the table in [gcp-posture.md](gcp-posture.md#supply-chain-and-organization-policy). Remaining: capture the outputs and the Recommender closing both P2s, with the verify block of [project-guardrails.md](../../runbooks/project-guardrails.md) | +| 6.2.1 | `gcloud org-policies describe run.allowedIngress --effective` on both projects, restricted to `internal-and-cloud-load-balancing` | `need` — unset (allow all) in both projects, measured 2026-10-27. It applies project-wide, so it can only be set once overfwd and the shortener stop running with `INGRESS_TRAFFIC_ALL`; see [gcp-posture.md](gcp-posture.md#cloud-run) | | The 18 DAST-validated requirements | An authenticated Burp Suite scan run with the ADA Burp Audit Scan Configuration. Developer-run at AL1; lab-run at AL2 | `need` — see [dast-readiness.md](dast-readiness.md) | diff --git a/docs/compliance/casa/gap-assessment.md b/docs/compliance/casa/gap-assessment.md index cadeaec9..a67dd377 100644 --- a/docs/compliance/casa/gap-assessment.md +++ b/docs/compliance/casa/gap-assessment.md @@ -210,7 +210,7 @@ magic link. | Req | Requirement | Verdict | Evidence / gap | Sev | |-----|-------------|---------|----------------|-----| -| 3.1.1 | Least-privilege access control on a trusted service layer | `pass` | Capability types in `crates/overslash-db/src/scopes/` (`SystemScope > OrgScope > UserScope > AgentScope`) inject `org_id` into every query. `/v1/actions/call` runs two layers: a default-deny group ceiling (`overslash-core/src/permissions/ceiling.rs`, enforced at `routes/actions/call.rs:346-356`) and an ancestor-chain walk with approval bubbling (`routes/actions/permission_gate.rs:48-90`). Enforcement is server-side only. **Infrastructure caveat from the live scan:** two GCP-default service accounts hold `roles/editor` on the production project — the default Compute Engine SA (flagged P2, **0 permissions used in 90 days**) and the cloudservices agent (P2, downgrade to `roles/compute.editor`) — both invisible to Terraform — [gcp-posture.md](gcp-posture.md). The scan also found the auto-created `overslash_cloudbuild` / `overslash-dev_cloudbuild` buckets under legacy ACLs (UBLA off, public-access prevention `inherited`), which gave project viewers read access to a bootstrap-era source archive that included a local-only settings file. **Fixed and applied 2026-10-23:** `infra/modules/cloud-build-source/` imports both, with UBLA on, prevention `enforced` and a 30-day delete rule. Re-measured: empty `acl`, no live objects. `.gcloudignore` keeps local-only files out of any future manual submit — [cloud-build-bucket.md](../../runbooks/cloud-build-bucket.md). The Terraform-managed IAM half is **applied too (2026-10-23)**. On dev, the scheduler's project-wide `roles/cloudsql.admin` is now the custom `overslash_dev_sql_start_stop` role, conditioned on `overslash-dev-db`, and no account holds `cloudsql.admin` in either project. The shared runtime account is split per workload: the API runs as `-run`, overfwd as `-overfwd`, the shortener as `-shortener` and the metrics exporter as `-exporter` ([least-privilege-iam.md](../../runbooks/least-privilege-iam.md)). The Cloud Build account's project-wide `iam.serviceAccountUser` remains | — | +| 3.1.1 | Least-privilege access control on a trusted service layer | `pass` | Capability types in `crates/overslash-db/src/scopes/` (`SystemScope > OrgScope > UserScope > AgentScope`) inject `org_id` into every query. `/v1/actions/call` runs two layers: a default-deny group ceiling (`overslash-core/src/permissions/ceiling.rs`, enforced at `routes/actions/call.rs:346-356`) and an ancestor-chain walk with approval bubbling (`routes/actions/permission_gate.rs:48-90`). Enforcement is server-side only. **Infrastructure caveat from the live scan:** two GCP-default service accounts held `roles/editor` on the production project — the default Compute Engine SA (flagged P2, **0 permissions used in 90 days**; dev too) and the cloudservices agent (P2, downgrade to `roles/compute.editor`) — both invisible to Terraform — [gcp-posture.md](gcp-posture.md). **In code, pending apply (prod); dev's binding removed by hand 2026-10-27:** `infra/modules/project-guardrails/` removes both with `google_project_iam_member_remove` (a re-added binding is plan drift), grants the cloudservices agent `compute.editor` only where the VPC connector needs it, and sets `iam.automaticIamGrantsForDefaultServiceAccounts` plus `iam.managed.preventPrivilegedBasicRolesForDefaultServiceAccounts` on the project, so the grants come back neither at creation nor by a later grant. Nothing ran as the default Compute SA (checked per service, job, trigger and scheduler job) — [project-guardrails.md](../../runbooks/project-guardrails.md). The scan also found the auto-created `overslash_cloudbuild` / `overslash-dev_cloudbuild` buckets under legacy ACLs (UBLA off, public-access prevention `inherited`), which gave project viewers read access to a bootstrap-era source archive that included a local-only settings file. **Fixed and applied 2026-10-23:** `infra/modules/cloud-build-source/` imports both, with UBLA on, prevention `enforced` and a 30-day delete rule. Re-measured: empty `acl`, no live objects. `.gcloudignore` keeps local-only files out of any future manual submit — [cloud-build-bucket.md](../../runbooks/cloud-build-bucket.md). The Terraform-managed IAM half is **applied too (2026-10-23)**. On dev, the scheduler's project-wide `roles/cloudsql.admin` is now the custom `overslash_dev_sql_start_stop` role, conditioned on `overslash-dev-db`, and no account holds `cloudsql.admin` in either project. The shared runtime account is split per workload: the API runs as `-run`, overfwd as `-overfwd`, the shortener as `-shortener` and the metrics exporter as `-exporter` ([least-privilege-iam.md](../../runbooks/least-privilege-iam.md)). The Cloud Build account's project-wide `iam.serviceAccountUser` remains | — | | 3.1.2 | Policy attributes not manipulable by end users | `pass` | **V2, fixed.** `POST /v1/api-keys` no longer accepts an `org_id`: `create_api_key` takes `AdminAcl` + the `OrgScope` extractor, so the org is the presented credential's currently-active org and the caller cannot name one (`routes/api_keys.rs`). `identity_id` is resolved through that scope. The unauthenticated bootstrap branch — the only reason a body `org_id` existed — is gone; the first admin key is minted by `POST /v1/orgs` (`routes/orgs/create.rs::provision_new_org_contents`) behind `ALLOW_ORG_CREATION`. Migration 121 backs it with a composite FK. Tests: `tests/api_key_org_binding.rs` | — | | 3.1.3 | Access controls fail securely | `pass` | Default-deny ceiling; foreign ids 404 at the SQL boundary (`routes/approvals/resolve.rs:31-34`); org-configurable response capture is fail-closed (`services/audit_capture.rs:44-47`) | — | | 3.1.4 | Protected against IDOR on create / read / update / delete | `pass` | The scope model holds across the surface — unscoped repo getters exist (`repos/service_template.rs:97`, `repos/mcp_upstream_connection.rs:79`, `repos/oauth_connection_flow.rs:119`) but every call site filters on the caller's org immediately. **V2 was the one exception — a create-path IDOR — and is fixed**: `routes/api_keys.rs` was the only handler in the routes tree taking its org from `req.*`, and it no longer does. Migration 121 adds the composite `api_keys (org_id, identity_id) -> identities (org_id, id)` FK so the pair cannot diverge even if a handler regresses | — | @@ -343,7 +343,7 @@ sensitive responses~~ — done, as the default on every API response that does n (`--provenance=false` at `infra/modules/cloud-build/main.tf:217`); ~~the `rsa` and `paste` advisories~~ — documented as time-boxed exceptions in [dependency-vulnerability-policy.md](dependency-vulnerability-policy.md), justified as -unreached and as unmaintained-with-no-fix, both of which the Test Guide explicitly permits; `db-f1-micro` sizing; an org-policy baseline; the three +unreached and as unmaintained-with-no-fix, both of which the Test Guide explicitly permits; `db-f1-micro` sizing; an org-policy baseline (now mostly in code at project level, pending apply — [project-guardrails.md](../../runbooks/project-guardrails.md); `run.allowedIngress` still open); the three production alert policies disabled at `infra/env/prod.tfvars:113-119`, which already carry their verification procedure inline and are exactly the shape of evidence a lab accepts. diff --git a/docs/compliance/casa/gcp-posture.md b/docs/compliance/casa/gcp-posture.md index fedbefbc..431f0b87 100644 --- a/docs/compliance/casa/gcp-posture.md +++ b/docs/compliance/casa/gcp-posture.md @@ -50,6 +50,29 @@ nothing of the sort. Both bindings are project-creation defaults, invisible to T and there is no `constraints/iam.automaticIamGrantsForDefaultServiceAccounts` to stop them being recreated. +**Remediation (2026-10-27).** Nothing depends on the default Compute SA: every Cloud Run +service and job, Cloud Build trigger and Scheduler job runs as its own account, there are no +instances, keys or `actAs` grants on it, and the one build ever run as it was the manual +submit of 2026-04-29. The cloudservices agent's 12 exercised permissions are all +`compute.instanceGroupManagers.*` / `instanceTemplates.*` / `instances.*` / `subnetworks.use`, +which is the prod VPC connector's managed instance group. Dev has no connector and its agent +holds no Editor. + +- Dev's default-SA binding was **removed by hand on 2026-10-27**, and `overslash-dev-api` + `/health` still returned 200. +- Prod was **applied on 2026-10-27**. `infra/modules/project-guardrails/` removes both + bindings with `google_project_iam_member_remove` and grants the cloudservices agent + `roles/compute.editor` first, but only where `use_private_vpc` (that is, where the + connector exists). Re-measured the same day: prod's only Editor-class binding is + `roles/compute.editor` on the cloudservices agent; dev has none. Prod `tofu plan` is clean. +- Both projects have since moved into the overspiral.com organization, which enforces + `iam.automaticIamGrantsForDefaultServiceAccounts` and + `iam.managed.preventPrivilegedBasicRolesForDefaultServiceAccounts` at org level. The + module also pins both at project level, so the grants can come back neither by creation + nor by a later grant, and a change of parent does not lift them. + +Operator step: [project-guardrails.md](../../runbooks/project-guardrails.md). + --- ## Measured configuration @@ -327,6 +350,47 @@ Organization policy constraints set on either project: **none**. all unset. Every guardrail in this stack is convention expressed in HCL, so a `gcloud` command or a console click moves the project outside it without failing anything. +**Update 2026-10-27: both projects now sit in the overspiral.com organization, and the +baseline is applied.** The organization enforces a policy set at org level, and +`infra/modules/project-guardrails/` ([project-guardrails.md](../../runbooks/project-guardrails.md)) +pins the Overslash-specific ones per project. Both applied on 2026-10-27; `tofu plan` is +clean in both environments. Effective policy, measured the same day with +`gcloud org-policies describe --project=

--effective`: + +| Constraint | `overslash` | `overslash-dev` | Set at | CASA | +|---|---|---|---|---| +| `iam.allowedPolicyMemberDomains` | org customer only | same | org | 3.1.1 | +| `iam.automaticIamGrantsForDefaultServiceAccounts` | enforced | enforced | org + project | 3.1.1 | +| `iam.managed.preventPrivilegedBasicRolesForDefaultServiceAccounts` | enforced | enforced | org + project | 3.1.1 | +| `iam.managed.disableServiceAccountKeyCreation` | enforced | **off** (Directory-sync key) | org; dev project opt-out | 6.7.1 | +| `iam.disableServiceAccountKeyUpload` | enforced | enforced | org | 6.7.1 | +| `iam.disableAuditLoggingExemption` | enforced | enforced | org + project | 6.7.1 | +| `sql.managed.restrictPublicIp` | enforced | **off** (public IP, no authorized networks) | org; dev project opt-out | 6.2.1 | +| `sql.managed.restrictAuthorizedNetworks` | enforced | **off** | org; dev project opt-out | 6.2.1 | +| `storage.publicAccessPrevention` | enforced | enforced | org + project | 6.2.1 | +| `storage.uniformBucketLevelAccess` | enforced | enforced | org | 6.2.1 | +| `storage.secureHttpTransport` | enforced | enforced | org | 4.1.1 | +| `compute.managed.requireOsLogin` | enforced | enforced | org | 3.1.1 | +| `compute.managed.disableSerialPortAccess` | enforced | enforced | org | 3.1.1 | +| `compute.skipDefaultNetworkCreation` | enforced | enforced | org | — | +| `compute.managed.restrictProtocolForwardingCreationForTypes` | `INTERNAL` only | same | org | — | +| `essentialcontacts.managed.allowedContactDomains` | `@overspiral.com` | same | org | — | +| `run.allowedIngress` | **unset** (allow all) | **unset** | — | 6.2.1 | + +So the closing sentence above no longer holds: a `gcloud` command or console click that +would make a bucket public, add a public IP or an authorized network to the prod database, +mint or upload a service-account key, grant a default SA a basic role, or add an IAM member +from outside the organization now **fails** instead of drifting silently. + +`iam.allowedPolicyMemberDomains` is why the public Cloud Run services moved to +`invoker_iam_disabled` instead of an `allUsers` binding. The old `allUsers` invoker +bindings were removed by hand on 2026-10-27; no Cloud Run service in either project has an +`allUsers` or `allAuthenticatedUsers` member, and `api.overslash.com/health`, the +shortener and `overslash-dev-api` still answer 200. + +**Still open:** `run.allowedIngress` — it applies project-wide, so it can only be tightened +once overfwd and the shortener stop running with `INGRESS_TRAFFIC_ALL`. + --- ### DNS — not in either project diff --git a/docs/runbooks/project-guardrails.md b/docs/runbooks/project-guardrails.md new file mode 100644 index 00000000..b9e0dda1 --- /dev/null +++ b/docs/runbooks/project-guardrails.md @@ -0,0 +1,177 @@ +# Project guardrails — default-SA Editor and project org policies + +`infra/modules/project-guardrails/main.tf` does two things in each project: + +1. **Takes `roles/editor` away from the two GCP-default accounts** that Google's IAM + Recommender flagged ([gcp-posture.md](../compliance/casa/gcp-posture.md#iam-policy-googleiampolicyrecommender-global)): + + | Member | Was | Becomes | Why | + |---|---|---|---| + | `-compute@developer.gserviceaccount.com` (default Compute Engine SA) | `roles/editor` | nothing | 0 permissions used in 90 days; nothing runs as it | + | `@cloudservices.gserviceaccount.com` (Google APIs service agent) | `roles/editor` | `roles/compute.editor`, **prod only** (`use_private_vpc`) | the 12 permissions it used are all the VPC connector's managed instance group | + + `google_project_iam_member_remove` keeps them removed. A binding someone re-adds by hand + shows up as a create on the next plan and is removed again on apply. + +2. **Sets project-level org policies.** Org-wide policies live in the org repo. These are the + ones that are Overslash-specific, or that pin a guarantee this project relies on: + + | Constraint | dev | prod | What it guards | + |---|---|---|---| + | `iam.automaticIamGrantsForDefaultServiceAccounts` | enforced | enforced | A default SA is never *created* with Editor. Already inherited from the org; pinned here so it survives a change of parent | + | `iam.managed.preventPrivilegedBasicRolesForDefaultServiceAccounts` | enforced | enforced | Nobody *grants* a default SA Editor/Owner later | + | `iam.disableAuditLoggingExemption` | enforced | enforced | The Data Access audit config cannot exempt a principal (6.7.1) | + | `storage.publicAccessPrevention` | enforced | enforced | No bucket can be made public. All three buckets are already `enforced` | + | `sql.managed.restrictPublicIp` | **off** | enforced | Prod's database stays private. Dev's has a public IP with no authorized networks | + | `sql.managed.restrictAuthorizedNetworks` | **off** | enforced | Same split | + | `iam.managed.disableServiceAccountKeyCreation` | **off** (`allow_service_account_key_creation`) | enforced | The org enforces it. Dev opts out so the Directory-sync key can be minted ([google-directory.md](google-directory.md)) | + + The dev key-creation override already exists. It was set by hand on 2026-10-27, and an + `import` block in `infra/main.tf` adopts it. + + The organization now enforces every "enforced" cell above at org level as well, so the + project-level copies are pins: they keep the guarantee if the project changes parent. + The dev "off" cells are real overrides of an org-level enforce. + +**Status: applied to dev and prod on 2026-10-27.** `tofu plan` is clean in both. + +`run.allowedIngress` is deliberately absent. The constraint applies to the whole project, +and overfwd and the shortener still run with `INGRESS_TRAFFIC_ALL`, so the only value that +fits today is `all`, which enforces nothing. + +Infra is never auto-applied. This page is the human half. + +## Before you apply + +- **Who applies.** Org policies need `orgpolicy.policy.set`. Project Owner does **not** + carry it, and `roles/orgpolicy.policyAdmin` can only be granted on the organization + (overspiral.com). Run tofu as an identity that holds it there. +- **Quota project.** With user ADC, `orgpolicy.googleapis.com` refuses calls that carry no + quota project. `infra/providers.tf` sets `user_project_override` + `billing_project`, so + every provider call is billed to the managed project and no local setup is needed. That + means `cloudresourcemanager.googleapis.com` must already be enabled there, which it is + on both projects. +- **Secret-read alert.** `terraform_operators` (tfvars) lists the principals whose + `tofu plan` secret reads do not page. If you apply as `angel@overspiral.com`, add it there + first, or expect an `AccessSecretVersion` alert per plan. +- **Nothing depends on the default Compute SA.** Measured 2026-10-27, both projects: + - Cloud Run services and jobs run as `overslash--{run,exporter,shortener,overfwd}`. + - Cloud Build triggers run as `overslash--build`. + - Scheduler jobs run as `overslash--scheduler`. + - There are no Compute Engine instances. + - The default SA has no user-managed keys and nobody holds `actAs` on it. + - The only build that ever ran as it was the manual `gcloud builds submit` of 2026-04-29. + + Re-check before applying: + + ```bash + P=overslash # then overslash-dev + N=$(gcloud projects describe $P --format='value(projectNumber)') + gcloud run services list --project $P --format='value(metadata.name,spec.template.spec.serviceAccountName)' + gcloud run jobs list --project $P --format='value(metadata.name,spec.template.spec.template.spec.serviceAccountName)' + gcloud builds triggers list --project $P --region europe-west1 --format='value(name,serviceAccount)' + gcloud scheduler jobs list --project $P --location europe-west1 \ + --format='value(name,httpTarget.oauthToken.serviceAccountEmail,httpTarget.oidcToken.serviceAccountEmail)' + gcloud compute instances list --project $P + gcloud iam service-accounts keys list --managed-by=user --iam-account $N-compute@developer.gserviceaccount.com + # expect: no "-compute@developer" anywhere, no instances, no keys + ``` + +- **Manual builds after this.** A `gcloud builds submit` must name its account: + `--service-account=projects/

/serviceAccounts/overslash--build@

.iam.gserviceaccount.com`. + The org's `cloudbuild.useBuildServiceAccount` already requires this. + +## Apply: dev first, then prod + +```bash +make tofu-init +make tofu-plan ENV=dev +# Expect, under module.project_guardrails: +# google_org_policy_policy.service_account_key_creation (imported, 0 to change) +# + 6 google_org_policy_policy +# + 2 google_project_iam_member_remove +# + google_project_service.apis["orgpolicy.googleapis.com"] +# no google_project_iam_member.cloudservices_compute_editor (dev has no VPC connector) +# If unrelated drift shows up and you don't want it now: +# cd infra && tofu workspace select dev && tofu plan -var-file=env/dev.tfvars \ +# -target=google_project_service.apis -target=module.project_guardrails -out=dev.tfplan && cd .. +make tofu-apply ENV=dev + +make tofu-plan ENV=prod +# Expect: 11 to add, 0 to change, 0 to destroy under the same targets. That is the dev set +# plus google_project_iam_member.cloudservices_compute_editor[0], and no import. +make tofu-apply ENV=prod # asks you to type `prod` +``` + +A read-only targeted plan was run on both workspaces before the PR. Prod showed exactly +`11 to add, 0 to change, 0 to destroy`. Dev showed 10 adds before the `import` block went in. +The sandbox cannot read the existing dev policy, so the operator's plan is the first to show +the import. + +**Order inside the apply.** Terraform grants `roles/compute.editor` before it removes Editor +from the cloudservices agent, so the VPC connector never loses its instance-group +permissions. It sets the `preventPrivilegedBasicRoles…` policy only after both removals. + +### Without Terraform (the Editor half only) + +Dev's default Compute SA binding was removed this way on 2026-10-27, and `overslash-dev-api` +kept serving (`/health` 200). The module's `member_remove` is a no-op against an +already-removed binding, so doing this first is safe. For prod: + +```bash +gcloud projects add-iam-policy-binding overslash \ + --member=serviceAccount:212015650448@cloudservices.gserviceaccount.com \ + --role=roles/compute.editor --condition=None +gcloud projects remove-iam-policy-binding overslash \ + --member=serviceAccount:212015650448@cloudservices.gserviceaccount.com \ + --role=roles/editor --condition=None +gcloud projects remove-iam-policy-binding overslash \ + --member=serviceAccount:212015650448-compute@developer.gserviceaccount.com \ + --role=roles/editor --condition=None +``` + +Grant before you remove, in that order. + +## Verify — evidence for the CASA pack + +```bash +for P in overslash-dev overslash; do + echo "== $P" + # 1. No Editor on either default account; compute.editor on cloudservices in prod only + gcloud projects get-iam-policy $P --flatten='bindings[].members' \ + --format='value(bindings.role,bindings.members)' | grep -E 'compute@developer|cloudservices' + # 2. The project-level policies + for C in iam.automaticIamGrantsForDefaultServiceAccounts \ + iam.managed.preventPrivilegedBasicRolesForDefaultServiceAccounts \ + iam.disableAuditLoggingExemption storage.publicAccessPrevention \ + sql.managed.restrictPublicIp sql.managed.restrictAuthorizedNetworks \ + iam.managed.disableServiceAccountKeyCreation; do + printf '%s ' $C + gcloud org-policies describe $C --project $P --format='value(spec.rules[0].enforce)' + done +done +# 3. Recommender closes the P2s once it re-observes (can take a day). The command needs a +# quota project where you hold serviceusage.services.use. +CLOUDSDK_BILLING_QUOTA_PROJECT=overslash-dev gcloud recommender recommendations list \ + --project overslash --location global --recommender google.iam.policy.Recommender \ + --format='value(priority,description,stateInfo.state)' +``` + +Expected: + +- **Step 1:** dev prints only `roles/compute.instanceGroupManagerServiceAgent` for + cloudservices. Prod prints that plus `roles/compute.editor`. Neither prints `roles/editor`. +- **Step 2:** every constraint prints `True`, except the three dev ones listed as off above, + which print `False`. +- **Step 3:** the two `roles/editor` recommendations are gone, or `SUCCEEDED`. +- **Smoke test:** `api.overslash.com` 200, and the shortener (which reaches Valkey through + the VPC connector) still resolves a link. + +## Rollback + +- **Editor half:** re-add the binding with `gcloud projects add-iam-policy-binding … --role=roles/editor` + and delete the module's `member_remove` resource, or the next apply removes it again. The + `preventPrivilegedBasicRoles…` policy blocks that re-grant, so set it to `FALSE` first. +- **A policy that blocks something legitimate:** flip its `enforce` in the module, or + `gcloud org-policies reset --project

` to fall back to what the org + inherits. diff --git a/infra/README.md b/infra/README.md index b7761d0d..67b242e9 100644 --- a/infra/README.md +++ b/infra/README.md @@ -149,6 +149,7 @@ the GitHub button on `/auth/providers` only once both are populated. | `api-lb` | (Optional, `enable_api_lb`) Global HTTPS LB for `` + `*.`: wildcard Certificate Manager cert, TLS 1.2+ `RESTRICTED` SSL policy, Cloud Armor (WAF in preview), optional IPv6 front door (`api_lb_enable_ipv6`) — see [docs/runbooks/lb-hardening.md](../docs/runbooks/lb-hardening.md) | | `cloud-build` | GitHub push trigger: build -> push -> deploy | | `cloud-build-source` | Adopts (via `import`) the auto-created `_cloudbuild` bucket: UBLA on, public access prevention enforced, 30-day object expiry. No trigger uses it — see [docs/runbooks/cloud-build-bucket.md](../docs/runbooks/cloud-build-bucket.md) | +| `project-guardrails` | Removes the GCP-default `roles/editor` grants (default Compute SA, cloudservices agent → `compute.editor` where the VPC connector exists) and sets project-level org policies — see [docs/runbooks/project-guardrails.md](../docs/runbooks/project-guardrails.md) | | `infra-scheduler` | (Optional) Stop/start Cloud SQL on cron (Europe/Madrid) | | `dns` | (Optional) Cloud DNS managed zone | | `memorystore` | (Optional) Memorystore for Valkey 9.1 over Private Service Connect, IAM auth + TLS on; the API and shortener SAs get `memorystore.dbConnectionUser` on the instance and the server CA is published to Secret Manager. `TF_VAR_cache_instance_name` swaps instances without downtime. See [docs/runbooks/memorystore-valkey.md](../docs/runbooks/memorystore-valkey.md) | diff --git a/infra/env/dev.tfvars b/infra/env/dev.tfvars index f818280a..c66b7c59 100644 --- a/infra/env/dev.tfvars +++ b/infra/env/dev.tfvars @@ -191,3 +191,7 @@ audit_log_bucket_locked = false enable_google_directory_sync = true +# The org enforces iam.managed.disableServiceAccountKeyCreation; dev opts out so +# the Directory-sync key can be minted (docs/runbooks/project-guardrails.md). +allow_service_account_key_creation = true + diff --git a/infra/main.tf b/infra/main.tf index ec52a2f4..1bd697dc 100644 --- a/infra/main.tf +++ b/infra/main.tf @@ -12,6 +12,11 @@ resource "google_project_service" "apis" { "monitoring.googleapis.com", "logging.googleapis.com", "billingbudgets.googleapis.com", + "orgpolicy.googleapis.com", + # The provider bills quota to this project (user_project_override), so + # its own reads go through here. Enable by hand on a fresh project: + # this list can't be read without it. + "cloudresourcemanager.googleapis.com", ], var.use_private_vpc ? [ "servicenetworking.googleapis.com", @@ -54,6 +59,27 @@ module "iam" { depends_on = [google_project_service.apis] } +# --- Project guardrails: default-SA Editor removal + project org policies +# (CASA 3.1.1; docs/runbooks/project-guardrails.md) --- +module "project_guardrails" { + source = "./modules/project-guardrails" + project_id = var.project_id + + vpc_connector_enabled = var.use_private_vpc + restrict_sql_public_ip = var.use_private_vpc + allow_service_account_key_creation = var.allow_service_account_key_creation + + depends_on = [google_project_service.apis] +} + +# Adopt the key-creation override set by hand on dev (2026-10-27). A no-op once +# it is in state; prod has no hand-set policy, so it imports nothing there. +import { + for_each = var.allow_service_account_key_creation ? toset(["adopt"]) : toset([]) + to = module.project_guardrails.google_org_policy_policy.service_account_key_creation + id = "projects/${var.project_id}/policies/iam.managed.disableServiceAccountKeyCreation" +} + # --- Artifact Registry --- module "artifact_registry" { source = "./modules/artifact-registry" diff --git a/infra/modules/cloud-run-overfwd/main.tf b/infra/modules/cloud-run-overfwd/main.tf index 44cc1832..0abf49fa 100644 --- a/infra/modules/cloud-run-overfwd/main.tf +++ b/infra/modules/cloud-run-overfwd/main.tf @@ -240,6 +240,10 @@ resource "google_cloud_run_v2_service" "overfwd" { # mid-apply; drop the leftover binding by hand afterwards. removed { from = google_cloud_run_v2_service_iam_member.public + + lifecycle { + destroy = false + } } resource "google_cloud_run_domain_mapping" "domain" { diff --git a/infra/modules/cloud-run-shortener/main.tf b/infra/modules/cloud-run-shortener/main.tf index 8900a48a..19457421 100644 --- a/infra/modules/cloud-run-shortener/main.tf +++ b/infra/modules/cloud-run-shortener/main.tf @@ -260,6 +260,10 @@ resource "google_cloud_run_v2_service" "shortener" { # mid-apply; drop the leftover binding by hand afterwards. removed { from = google_cloud_run_v2_service_iam_member.public + + lifecycle { + destroy = false + } } resource "google_cloud_run_domain_mapping" "domain" { diff --git a/infra/modules/cloud-run/main.tf b/infra/modules/cloud-run/main.tf index 7bdef3aa..06df2aa8 100644 --- a/infra/modules/cloud-run/main.tf +++ b/infra/modules/cloud-run/main.tf @@ -769,6 +769,10 @@ resource "google_cloud_run_v2_service" "api" { # mid-apply; drop the leftover binding by hand afterwards. removed { from = google_cloud_run_v2_service_iam_member.public + + lifecycle { + destroy = false + } } resource "google_cloud_run_domain_mapping" "domain" { diff --git a/infra/modules/project-guardrails/main.tf b/infra/modules/project-guardrails/main.tf new file mode 100644 index 00000000..c53a3acb --- /dev/null +++ b/infra/modules/project-guardrails/main.tf @@ -0,0 +1,173 @@ +# Project-level guardrails (CASA 3.1.1, 6.2.1, 6.7.1; docs/compliance/casa/gcp-posture.md). +# +# Two halves: +# +# 1. The `roles/editor` grants GCP made at project creation, which Google's IAM +# Recommender flagged and nothing else in this repo owns: +# - the default Compute Engine SA: 0 permissions used in 90 days, and +# nothing runs as it (every Cloud Run service/job, Cloud Build trigger +# and Scheduler job names its own SA) -> removed. +# - the Google APIs service agent (`@cloudservices`): the 12 permissions it +# used are all the Serverless VPC Access connector's managed instance +# group -> `roles/compute.editor`, only where that connector exists. +# `google_project_iam_member_remove` keeps them removed: a binding re-added +# out-of-band shows up as drift on the next plan and is removed again. +# +# 2. Organization policies set on the project, so the posture lives in this +# repo rather than depending on what the parent org happens to inherit. +# Org-wide policies belong to the org repo; these are the ones whose value +# is Overslash-specific (per-env exceptions) or that pin a guarantee this +# project relies on. +# +# Applying needs `roles/orgpolicy.policyAdmin` on the org, which Owner on the +# project does not carry. Runbook: docs/runbooks/project-guardrails.md. + +variable "project_id" { + type = string +} + +variable "vpc_connector_enabled" { + type = bool + description = "Whether a Serverless VPC Access connector exists (use_private_vpc). Its managed instance group is what the Google APIs service agent needs compute.editor for." +} + +variable "restrict_sql_public_ip" { + type = bool + description = "Enforce sql.managed.restrictPublicIp and sql.managed.restrictAuthorizedNetworks. False on dev, whose database has a public IP with no authorized networks." +} + +variable "allow_service_account_key_creation" { + type = bool + description = "Turn iam.managed.disableServiceAccountKeyCreation off for this project. The org enforces it; dev opts out so the Directory-sync key can be minted there." +} + +data "google_project" "current" { + project_id = var.project_id +} + +locals { + compute_default_sa = "serviceAccount:${data.google_project.current.number}-compute@developer.gserviceaccount.com" + cloudservices = "serviceAccount:${data.google_project.current.number}@cloudservices.gserviceaccount.com" +} + +# --- Default-SA Editor --- + +resource "google_project_iam_member_remove" "compute_default_sa_editor" { + project = var.project_id + role = "roles/editor" + member = local.compute_default_sa +} + +# Granted before Editor is taken away, so the connector never loses the +# instance-group permissions mid-apply. +resource "google_project_iam_member" "cloudservices_compute_editor" { + count = var.vpc_connector_enabled ? 1 : 0 + + project = var.project_id + role = "roles/compute.editor" + member = local.cloudservices +} + +resource "google_project_iam_member_remove" "cloudservices_editor" { + project = var.project_id + role = "roles/editor" + member = local.cloudservices + + depends_on = [google_project_iam_member.cloudservices_compute_editor] +} + +# --- Organization policies --- + +# Default SAs are never created with Editor. Inherited from the org today; +# pinned here so it survives a change of parent. +resource "google_org_policy_policy" "automatic_iam_grants" { + name = "projects/${var.project_id}/policies/iam.automaticIamGrantsForDefaultServiceAccounts" + parent = "projects/${var.project_id}" + + spec { + rules { + enforce = "TRUE" + } + } +} + +# ...and no one grants them Editor/Owner later. The constraint above only acts +# when a default SA is created; this one acts on every IAM policy write. Set +# after the removals so it never judges a policy that still holds the grants. +resource "google_org_policy_policy" "no_basic_roles_for_default_sas" { + name = "projects/${var.project_id}/policies/iam.managed.preventPrivilegedBasicRolesForDefaultServiceAccounts" + parent = "projects/${var.project_id}" + + spec { + rules { + enforce = "TRUE" + } + } + + depends_on = [ + google_project_iam_member_remove.compute_default_sa_editor, + google_project_iam_member_remove.cloudservices_editor, + ] +} + +# The Data Access audit config (modules/audit-logging) cannot be quietly +# narrowed by exempting a principal. +resource "google_org_policy_policy" "no_audit_logging_exemption" { + name = "projects/${var.project_id}/policies/iam.disableAuditLoggingExemption" + parent = "projects/${var.project_id}" + + spec { + rules { + enforce = "TRUE" + } + } +} + +# No bucket can be made public. Every bucket here is already `enforced`. +resource "google_org_policy_policy" "storage_public_access_prevention" { + name = "projects/${var.project_id}/policies/storage.publicAccessPrevention" + parent = "projects/${var.project_id}" + + spec { + rules { + enforce = "TRUE" + } + } +} + +# Set explicitly in both directions, so an org-level change cannot flip +# either environment. +resource "google_org_policy_policy" "sql_restrict_public_ip" { + name = "projects/${var.project_id}/policies/sql.managed.restrictPublicIp" + parent = "projects/${var.project_id}" + + spec { + rules { + enforce = var.restrict_sql_public_ip ? "TRUE" : "FALSE" + } + } +} + +resource "google_org_policy_policy" "sql_restrict_authorized_networks" { + name = "projects/${var.project_id}/policies/sql.managed.restrictAuthorizedNetworks" + parent = "projects/${var.project_id}" + + spec { + rules { + enforce = var.restrict_sql_public_ip ? "TRUE" : "FALSE" + } + } +} + +# The dev override already exists (set by hand on 2026-10-27); the runbook +# imports it before the first apply. +resource "google_org_policy_policy" "service_account_key_creation" { + name = "projects/${var.project_id}/policies/iam.managed.disableServiceAccountKeyCreation" + parent = "projects/${var.project_id}" + + spec { + rules { + enforce = var.allow_service_account_key_creation ? "FALSE" : "TRUE" + } + } +} diff --git a/infra/providers.tf b/infra/providers.tf index fe54fd30..1fa52655 100644 --- a/infra/providers.tf +++ b/infra/providers.tf @@ -30,9 +30,21 @@ terraform { provider "google" { project = var.project_id region = var.region + + # Bill API quota to the managed project instead of the ADC client's + # project. Org Policy (project-guardrails) rejects user ADC calls + # without a quota project. + user_project_override = true + billing_project = var.project_id } provider "google-beta" { project = var.project_id region = var.region + + # Bill API quota to the managed project instead of the ADC client's + # project. Org Policy (project-guardrails) rejects user ADC calls + # without a quota project. + user_project_override = true + billing_project = var.project_id } diff --git a/infra/variables.tf b/infra/variables.tf index 754a6892..2a0d90a9 100644 --- a/infra/variables.tf +++ b/infra/variables.tf @@ -201,6 +201,12 @@ variable "enable_google_directory_sync" { default = false } +variable "allow_service_account_key_creation" { + description = "Opt this project out of the org's iam.managed.disableServiceAccountKeyCreation, so a Directory-sync key can be minted with gcloud (docs/runbooks/google-directory.md). Applied by modules/project-guardrails; docs/runbooks/project-guardrails.md." + type = bool + default = false +} + variable "enable_bi" { description = "Provision the BI surface: a BigQuery connection + overslash_bi dataset over the Postgres `bi` schema (docs/runbooks/bi.md)." type = bool From 9376ce7d632c0056674e950d5b048c1ea31e0ac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81ngel=20Manuel=20Mart=C3=ADn?= Date: Wed, 28 Oct 2026 00:25:56 +0100 Subject: [PATCH 04/10] feat(infra): prod uptime check + [P0] API Down; /health fails on DB down, TCP probes (#843) * feat(infra): prod uptime check + [P0] API Down, enable PromQL alert gates Prod had no GCM uptime check and no API-down page: the monitoring module gates both on api_domain, which main.tf fed from var.domain, and prod leaves domain empty because the GCLB serves api_host_suffix. Fall back to api_host_suffix when the LB is on. The three PromQL gates (api_latency, oauth_refresh, upstream_error) are on in both envs; their descriptors were measured present 2026-10-27. The OAuth, upstream and webhook ratio queries clamped a per-second denominator to 1, which at our volume made them unable to fire; they now use a 0.001 floor plus a minimum-sample guard, as the latency alert already did. Same fix on the two Actions & OAuth dashboard ratio charts. Runbook: docs/runbooks/uptime-and-alerts.md. Not applied. Co-Authored-By: Claude Opus 5.5 * fix(api): /health 503s when Postgres is down; Cloud Run probes are TCP The HTTP liveness probe on /health (1s default timeout) lost the race with the 2s DB check once Cloud SQL went away, so dev's instance was shut down ~90s into every nightly stop and no replacement could boot: Cloud Run answered 500 until morning. The always-200 /health existed only to protect those probes, and it did not. Probes now open a TCP socket on the container port (the listener binds only after create_app has connected, migrated and preflighted, so startup loses nothing). /health shares /ready's gate: 200 ok, or 503 degraded when the DB probe fails, so [P0] API Down covers the database too. Dev's nightly API Down is accepted. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Factory Co-authored-by: Claude Opus 5.5 --- DECISIONS.md | 5 + SPEC.md | 2 +- STATUS.md | 2 +- TODO.md | 2 +- crates/overslash-api/src/routes/health.rs | 61 ++++--- crates/overslash-api/tests/health.rs | 13 +- docs/compliance/casa/gcp-posture.md | 59 +++++-- docs/runbooks/status-page.md | 32 ++-- docs/runbooks/uptime-and-alerts.md | 163 ++++++++++++++++++ infra/env/dev.tfvars | 18 +- infra/env/prod.tfvars | 18 +- infra/main.tf | 15 +- infra/modules/cloud-run/main.tf | 14 +- infra/modules/monitoring/alerts_p1.tf | 52 ++++-- .../dashboards/actions-and-oauth.json.tftpl | 4 +- 15 files changed, 348 insertions(+), 112 deletions(-) create mode 100644 docs/runbooks/uptime-and-alerts.md diff --git a/DECISIONS.md b/DECISIONS.md index 9acd6315..0a7eb873 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1537,3 +1537,8 @@ The disclosure input parses a form's body as JSON, as it does for a JSON body. **Decision**: For Overslash-managed sign-in (no `org_idp_configs` row claims the provider), login uses the deployment's `{PROVIDER}_AUTH_*` app even when the org has saved OAuth App Credentials (`OAUTH_{PROVIDER}_CLIENT_ID/SECRET` org secrets) for that provider. Those credentials still override env for service connections (SPEC §7). An admin can opt a provider in: a row in `org_oauth_signin_overrides (org_id, provider_key)`, off by default, makes the org's credentials serve sign-in too. It is set from the *Use for sign-in* action on a Google/GitHub row in Org Settings → OAuth App Credentials (`PATCH /v1/org-oauth-credentials/{provider} {override_signin}`), shown only when the org override exists and the env sign-in app is configured, or from the same checkbox on the save form (`PUT … {override_signin}`). Deleting the credentials clears the opt-in. Dedicated IdP rows, including those with `use_org_credentials`, are unchanged: they are already an explicit sign-in choice. There is no backfill, so orgs that had saved Google/GitHub credentials go back to the built-in sign-in app until an admin opts in. `services::org_signin::resolve_org_signin_credentials`; tests in `tests/org_oauth_credentials.rs`. **Rationale**: Admins save org OAuth credentials to brand or scope service connections, and the old precedence quietly moved every member's login onto that app as well. That app may have a narrower consent screen, test-user limits or unverified status, so login could break for reasons unrelated to sign-in. Making the sign-in takeover a separate, visible opt-in keeps "configure connections" from changing "how people log in". +## D-NEXT: `/health` fails when Postgres is down; Cloud Run probes are TCP + +**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. diff --git a/SPEC.md b/SPEC.md index 049431b4..b9978a2a 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1852,7 +1852,7 @@ Every per-IP throttle — these, and the in-handler magic-link, download and upl - **Storage**: Redis/Valkey if available (distributed, accurate across instances); in-memory `DashMap` fallback (single-instance, no external dependency). - **Fail-open**: If Redis becomes unavailable at runtime, requests are allowed through (logged as warning). - **Health endpoint** (`/health`) and **build stamp** (`GET /v1/version`) are exempt from rate limiting. -- **Health vs. readiness**: `/health` is liveness — it reports database reachability in the body (`db`, plus `db_latency_ms` when up; the error itself is logged, never returned, since both endpoints are unauthenticated) but always returns 200, because it backs the Cloud Run startup and liveness probes and a 503 there would restart containers during a database outage. `/ready` runs the same bounded `SELECT 1` and returns **503** when Postgres is unreachable; it is the endpoint to point a load balancer or alerting monitor at. +- **Health vs. liveness**: `/health` is origin health — it runs a bounded `SELECT 1` and returns 200 (`status: ok`) only when Postgres answers, **503** (`status: degraded`) otherwise, reporting `db` (plus `db_latency_ms` when up) in the body; the error itself is logged, never returned, since the endpoint is unauthenticated. It is what the uptime check and the status page probe. `/ready` applies the same gate (`status: ready`). Liveness is not an HTTP endpoint at all: the Cloud Run startup and liveness probes are TCP probes on the container port, so a database outage fails `/health` without restarting containers (D-NEXT). - **Build stamp**: both `/health` and `/ready` also report `version` (release, or the crate version for unreleased builds) and `commit` (full git SHA, or `unknown`). `GET /v1/version` returns the same two values plus `commit_short`, without the database probe — it is unauthenticated for the same reason `/health` is, and is what the dashboard renders in its sidebar footer. The SHA is baked in at compile time (`OVERSLASH_GIT_SHA`, supplied by Cloud Build as `$COMMIT_SHA` or discovered via `git` for local builds). --- diff --git a/STATUS.md b/STATUS.md index 9b8a83ea..a5cadf27 100644 --- a/STATUS.md +++ b/STATUS.md @@ -230,7 +230,7 @@ ### Monitoring & Observability -- OpenTofu module `infra/modules/monitoring/` (PRs #200, #205, #207, #272) deploys 5 GCP dashboards — `overview`, `api-use`, `actions-and-oauth`, `cloudsql-use`, `business` — plus P0/P1/P2 alert policies: API down, API 5xx > 1%, gateway slow-request ratio, Cloud SQL CPU/disk, background-task staleness, OAuth refresh failure ratio, webhook terminal failure ratio, plus uptime checks. +- OpenTofu module `infra/modules/monitoring/` (PRs #200, #205, #207, #272) deploys 5 GCP dashboards — `overview`, `api-use`, `actions-and-oauth`, `cloudsql-use`, `business` — plus P0/P1/P2 alert policies. **Deployed in both envs (measured 2026-10-27, 14 policies + 1 uptime check each):** API down on the `/health` uptime check + API 5xx > 1% (P0); API CPU/memory, gateway slow-request ratio, Cloud SQL CPU/disk, background-task staleness, OAuth refresh failure ratio, upstream error ratio, webhook terminal failure ratio, data integrity, unexpected secret access (P1); API 4xx (P2). Prod P0s page PagerDuty (EU) + email. Dev's API Down fires every night during the Cloud SQL stop, by design. Runbook: [uptime-and-alerts.md](docs/runbooks/uptime-and-alerts.md). - OTel sidecar exports metrics into GMP (instance-label collision fixed in #272). - **Data integrity sweep.** Every metrics-exporter tick also runs `overslash_db::integrity::sweep`: 15 read-only SQL invariants (`crates/overslash-db/src/integrity/*.sql`) over stored references the binding policies (D119, D122) and the relationship gates (#723, #737) would refuse — secret bindings outside the owner's or org vault, bare bindings, pins to a colleague's or another org's connection, pins on org-level instances, BYOC clients across users/orgs/providers, agent-owned secrets, cross-org owners and identity chains, templates from another tier or org, and pending approvals whose resolver is in another org or off the requester's chain. Each emits `custom.googleapis.com/overslash/business/integrity_violations{invariant}` (zeros included) and logs offending rows' ids at WARN; a failing sweep is skipped without losing the business metrics. `[P1] Data Integrity Violation` (any point > 0, or the series absent 30 min) is **gated off** behind `integrity_alert_enabled` until the metric descriptor exists; the pin↔template provider check is not swept (provider isn't in SQL). Runbook: [`docs/runbooks/data-integrity.md`](docs/runbooks/data-integrity.md). - The gateway slow-request alert is a PromQL ratio over diff --git a/TODO.md b/TODO.md index 60035b06..1dc2f424 100644 --- a/TODO.md +++ b/TODO.md @@ -120,7 +120,7 @@ are gaps today. - [ ] Enable container image vulnerability scanning (`containerscanning.googleapis.com`); none of Container Scanning, Container Analysis, Binary Authorization or Security Command Center is enabled. Pairs with the CI-side 6.1.1 work. - [ ] Secret Manager: 16 secrets, none with `rotation`, `next_rotation_time` or `expire_time`. Nothing schedules or alerts on rotation age. (CASA 6.7.1) - [x] **Memorystore has neither AUTH nor transit encryption** — `overslash-prod-valkey` confirmed live: `authEnabled` unset, `transitEncryptionMode: DISABLED`, `BASIC` tier. **In code, pending apply:** `infra/modules/memorystore/main.tf` sets `auth_enabled = true` and `transit_encryption_mode = "SERVER_AUTHENTICATION"`, and publishes the AUTH string and server CA to Secret Manager (`overslash--valkey-auth`, `-valkey-server-ca`), which reach the API (`REDIS_PASSWORD`, `REDIS_CA_CERT`) and oversla-sh (`VALKEY_*`) by `secret_key_ref`. Both clients connect `rediss://` and verify the server against only that CA (`services/valkey.rs`, `oversla-sh/src/storage.rs`). **Operator step still open:** TLS forces a replacement of the instance (downtime, empty keyspace, oversla.sh links lost unless exported) — pre-flight, apply and verify per [memorystore-auth-tls.md](docs/runbooks/memorystore-auth-tls.md). (CASA 6.7.1) -- [ ] **Production has 0 uptime checks and no `[P0] API Down` alert** — 8 alert policies are deployed, which is exactly 12 declared minus 3 disabled in tfvars minus the API-down policy `count`-gated on `api_domain != ""`. The only deployed P0 is `API High 5xx Rate`, which cannot fire when the service returns nothing at all. +- [x] **Production has 0 uptime checks and no `[P0] API Down` alert** — 8 alert policies were deployed (10 on 2026-10-27), which was exactly 12 declared minus 3 disabled in tfvars minus the API-down policy `count`-gated on `api_domain != ""`. The only deployed P0 is `API High 5xx Rate`, which cannot fire when the service returns nothing at all. Root cause: `infra/main.tf` fed the gate `var.domain`, which prod leaves empty because the GCLB serves `api_host_suffix` instead of a domain mapping. It now falls back to `api_host_suffix` when the LB is on, which creates `overslash-prod-api-health` on `api.overslash.com/health` and `[P0] API Down` (PagerDuty + email). The 3 disabled gates (`api_latency`, `oauth_refresh`, `upstream_error`) are on in both envs: their descriptors were measured present on 2026-10-27. The OAuth, upstream and already-deployed webhook ratios clamped a per-second denominator to 1, so at prod volume they could never fire. They now use a `0.001` floor + a minimum-sample guard, like the latency alert. Better Stack (status.overslash.com) still probes the same URL, but it is console-managed and does not page. **Applied 2026-10-27, both envs**, verified per [uptime-and-alerts.md](docs/runbooks/uptime-and-alerts.md): prod has 14 policies (`[P0] API Down` on PagerDuty + email) and `overslash-prod-api-health` passing from all 6 checker regions. Found on the way: dev's `/health` 500'd every night, because the HTTP liveness probe on `/health` (1 s default timeout against a 2 s DB check) killed the instance about 90 s after the Cloud SQL stop, and no replacement could boot. Cloud Run probes are now TCP, and `/health` returns 503 when Postgres is down (D-NEXT), so the prod page covers the database too. **That part is in code, pending a Cloud Run apply + an API deploy.** - [x] **No SSL policy and no Cloud Armor on the production LB** — both `gcloud compute ssl-policies list` and `security-policies list` are empty. TLS 1.2/1.3-only is currently true but unpinned and uncitable. `infra/modules/api-lb/main.tf` now declares `overslash-prod-api-ssl-policy` (TLS 1.2 minimum, `RESTRICTED`) on the HTTPS proxy and `overslash-prod-api-armor` on the backend service — default allow, OWASP CRS rule sets in preview (logged, never blocked) until tuned. **Not applied** — in code, pending a human run of [lb-hardening.md](docs/runbooks/lb-hardening.md), which also lists the evidence commands. (CASA 4.1.1) - [x] Set the org-policy baseline that would make the above guardrails rather than conventions. **Applied 2026-10-27** in both projects. The overspiral.com org now enforces a baseline at org level (measured table in [gcp-posture.md](docs/compliance/casa/gcp-posture.md#supply-chain-and-organization-policy)), and `infra/modules/project-guardrails/` ([project-guardrails.md](docs/runbooks/project-guardrails.md)) pins the Overslash-specific ones per project: `sql.managed.restrictPublicIp` + `restrictAuthorizedNetworks` (enforced in prod, explicitly off on dev's public-IP database), `storage.publicAccessPrevention` and `iam.disableAuditLoggingExemption` (both projects). `iam.managed.disableServiceAccountKeyCreation` is inherited from the org, and dev's hand-set opt-out for the Directory-sync key is adopted into Terraform. Org-wide policies belong to the org repo, not here. - [ ] **`run.allowedIngress`** — unset (allow all) on both projects, measured 2026-10-27. It applies project-wide, so it can only be set to `internal-and-cloud-load-balancing` once overfwd and the shortener stop running with `INGRESS_TRAFFIC_ALL` (prod API is already LB-only). (CASA 6.2.1) diff --git a/crates/overslash-api/src/routes/health.rs b/crates/overslash-api/src/routes/health.rs index bc30e69c..a97c3f42 100644 --- a/crates/overslash-api/src/routes/health.rs +++ b/crates/overslash-api/src/routes/health.rs @@ -10,9 +10,8 @@ use crate::AppState; /// Upper bound on how long a probe may wait for Postgres. /// /// Without this, `PgPool::acquire` blocks for the pool's `acquire_timeout` -/// (30 s by default) when the database is unreachable — long enough for the -/// Cloud Run liveness probe to time out and recycle the container, which is -/// exactly what the always-200 `/health` below exists to prevent. +/// (30 s by default) when the database is unreachable, and the uptime check +/// (10 s timeout) would read a hung database as a timeout rather than a 503. const PROBE_TIMEOUT: Duration = Duration::from_secs(2); // The `version` / `commit` / `sql_policy` fields in both bodies are @@ -87,39 +86,27 @@ async fn probe_db(pool: &PgPool) -> DbProbe { } } -/// Liveness. **Always 200**, even when Postgres is unreachable. +/// Origin health: 200 when the process *and* Postgres are up, 503 otherwise. /// -/// This endpoint backs the Cloud Run startup *and* liveness probes -/// (`infra/modules/cloud-run/main.tf`) plus the Better Stack P0 monitor. If it -/// failed on a Cloud SQL blip, Cloud Run would kill and restart every container -/// mid-outage and the startup probe would block redeploys until the database -/// recovered — the probe would amplify the incident instead of reporting it. -/// -/// So DB state is reported in the body (`db`, plus `db_latency_ms` when up) and -/// never in the status code. For a check that *fails* when the database is -/// down, use [`ready`]. -async fn health(State(state): State) -> Json { - let probe = probe_db(&state.db).await; - let info = build_info(); - let mut body = json!({ - "status": "ok", - "version": info.version, - "commit": info.commit, - "sql_policy": overslash_core::sql_policy::available(), - }); - probe.extend(&mut body); - Json(body) +/// This is what the GCM uptime check (`[P0] API Down`) and the Better Stack +/// monitor probe, so a database outage pages. It used to be always-200 because +/// the Cloud Run probes pointed here; they are TCP probes now +/// (`infra/modules/cloud-run/main.tf`), so nothing restarts a container on a +/// 503 from this endpoint. +async fn health(State(state): State) -> (StatusCode, Json) { + respond(probe_db(&state.db).await, "ok") } -/// Readiness — 503 when Postgres is unreachable. -/// -/// The gated counterpart to [`health`]: this is the one safe to point a -/// load-balancer or an alerting monitor at, because failing it takes the -/// instance out of rotation rather than restarting it. +/// Readiness. Same gate as [`health`]; kept for the callers that use it. async fn ready(State(state): State) -> (StatusCode, Json) { - let probe = probe_db(&state.db).await; + respond(probe_db(&state.db).await, "ready") +} + +/// Shared body + status for both endpoints: `up_status` when the database +/// answered, 503 `degraded` when it did not. +fn respond(probe: DbProbe, up_status: &str) -> (StatusCode, Json) { let (code, status) = if probe.is_up() { - (StatusCode::OK, "ready") + (StatusCode::OK, up_status) } else { (StatusCode::SERVICE_UNAVAILABLE, "degraded") }; @@ -177,6 +164,18 @@ mod tests { ); } + #[test] + fn respond_is_503_degraded_when_db_is_down() { + let (code, Json(body)) = respond(DbProbe::Down, "ok"); + assert_eq!(code, StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(body["status"], "degraded"); + assert_eq!(body["db"], "down"); + + let (code, Json(body)) = respond(DbProbe::Up { latency_ms: 1 }, "ok"); + assert_eq!(code, StatusCode::OK); + assert_eq!(body["status"], "ok"); + } + #[test] fn extend_reports_down_without_detail() { let mut body = json!({ "status": "degraded" }); diff --git a/crates/overslash-api/tests/health.rs b/crates/overslash-api/tests/health.rs index ccb7f0f7..a6bba090 100644 --- a/crates/overslash-api/tests/health.rs +++ b/crates/overslash-api/tests/health.rs @@ -1,14 +1,11 @@ //! Integration tests for `/health` and `/ready`. //! -//! These pin the split that the endpoints exist to encode: +//! Both return 200 when Postgres answers and 503 `degraded` when it does not; +//! `/health` is what the uptime check and Better Stack probe. Cloud Run's own +//! probes are TCP (`infra/modules/cloud-run/main.tf`), so neither endpoint can +//! restart a container. //! -//! 1. `/health` is liveness. It reports database state in the *body* and -//! always returns 200, because Cloud Run's startup and liveness probes -//! point at it — a 503 there would recycle containers mid-outage and -//! block redeploys (see `infra/modules/cloud-run/main.tf`). -//! 2. `/ready` is readiness. It returns 503 when Postgres is unreachable. -//! -//! The unreachable-database branch is covered by a unit test in +//! The unreachable-database branch is covered by unit tests in //! `src/routes/health.rs`, which can point a lazy pool at a dead port without //! disturbing the Postgres instance this suite shares across threads. diff --git a/docs/compliance/casa/gcp-posture.md b/docs/compliance/casa/gcp-posture.md index 431f0b87..22343126 100644 --- a/docs/compliance/casa/gcp-posture.md +++ b/docs/compliance/casa/gcp-posture.md @@ -290,23 +290,52 @@ age, which is the shape of evidence 6.7.1 asks for. ### Monitoring — the count-gate, confirmed +| | 2026-09-22 | 2026-10-27 | +|---|---|---| +| Alert policies deployed (prod) | **8**, all enabled | **10**, all enabled | +| Uptime checks deployed (prod) | **0** | **0** | +| Uptime checks deployed (dev) | 1 | 1 (`overslash-dev-api-health` → `api.dev.overslash.com/health`) | + +The 2026-09-22 arithmetic matched `infra/` exactly: 12 policies declared, minus the 3 +disabled in `infra/env/prod.tfvars` (`api_latency`, `oauth_refresh`, `upstream_error`), +minus `[P0] API Down`, which is `count`-gated on `api_domain != ""` while prod sets +`domain = ""`. **So production has no GCM uptime check and no API-down page**. The only +deployed P0 is `API High 5xx Rate`, which cannot fire when the service is returning +nothing at all. Better Stack ([status-page.md](../../runbooks/status-page.md)) detects the +outage out-of-band, but it is console-managed, outside the reviewable configuration, and +does not page. The two policies added since are `[P1] Data Integrity Violation` and +`[P1] Unexpected Secret Access` (audit-logging module). + +Deployed on 2026-10-27: `[P0] API High 5xx Rate`, `[P1]` API High CPU / API High Memory / +Cloud SQL High CPU / Cloud SQL High Disk / Background Task Stale / Webhook Delivery Failure +Rate / Data Integrity Violation / Unexpected Secret Access, `[P2] API High 4xx Rate`. Both +notification channels exist and are enabled: `overslash-prod PagerDuty (EU)` +(`webhook_tokenauth`) and `overslash-prod Email Alerts`. The 5xx P0 routes to both. + +**The three disabled gates no longer have a reason to stay disabled.** Their descriptors +were measured present in both projects on 2026-10-27: +`overslash_http_request_duration_seconds/histogram`, `overslash_oauth_events_total/counter`, +`overslash_upstream_responses_total/counter`. Reading the queries turned up a second +defect: the OAuth, upstream and (deployed) webhook ratios divided by +`clamp_min(rate, 1)`, which at prod volume (well under 1/s) pins the denominator to 1. They +could not fire, so the deployed webhook policy was counted as coverage it did not provide. + +**Remediation, applied 2026-10-27 on both projects** ([uptime-and-alerts.md](../../runbooks/uptime-and-alerts.md)): +`infra/main.tf` feeds the monitoring module `api_host_suffix` (`api.overslash.com`, served +by the GCLB) when `domain` is empty and the LB is on. That creates the uptime check and +`[P0] API Down` in prod. All three gates are `true` in both tfvars, and the ratio queries +take a `0.001` divide-by-zero floor plus an explicit minimum-sample guard. + +Re-measured after the apply (2026-10-27, prod): + | | | |---|---| -| Alert policies deployed | **8**, all enabled | -| Uptime checks deployed | **0** | - -The arithmetic matches `infra/` exactly: 12 policies declared, minus the 3 disabled at -`infra/env/prod.tfvars:113-119` (`api_latency`, `oauth_refresh`, `upstream_error`), minus -`[P0] API Down`, which is `count`-gated on `api_domain != ""` while prod sets -`domain = ""`. **So production has no GCM uptime check and no API-down page**; the only -deployed P0 is `API High 5xx Rate`, which cannot fire when the service is returning -nothing at all. Better Stack covers the detection out-of-band, but it is console-managed -and outside the reviewable configuration. +| Alert policies deployed | **14**, all enabled. `[P0] API Down` and `[P0] API High 5xx Rate` route to both channels | +| Uptime checks deployed | **1**: `overslash-prod-api-health` → `api.overslash.com/health`, 60s. `check_passed` true from all 6 regions (usa-virginia, usa-oregon, usa-iowa, eur-belgium, apac-singapore, sa-brazil-sao_paulo) | -Deployed: `[P0] API High 5xx Rate`, `[P1]` API High CPU / API High Memory / Cloud SQL High -CPU / Cloud SQL High Disk / Background Task Stale / Webhook Delivery Failure Rate, -`[P2] API High 4xx Rate`. Every one is an availability or capacity signal — there is no -alert on any security event. +Follow-up, in code, pending a Cloud Run apply and an API deploy: `/health` returns 503 +when Postgres is down, and the Cloud Run probes are TCP, so `[P0] API Down` also covers +the database (D-NEXT in DECISIONS.md). ### Storage — production @@ -443,7 +472,7 @@ No verdict is overturned. It does three things: service accounts; the unmanaged `_cloudbuild` bucket holding repository source archives without uniform bucket-level access (fixed and applied 2026-10-23 — [cloud-build-bucket.md](../../runbooks/cloud-build-bucket.md)); and the confirmation that production runs 8 alert - policies and **zero** uptime checks, so the `[P0] API Down` page does not exist. + policies and **zero** uptime checks, so the `[P0] API Down` page does not exist. (Root cause and fix, pending apply: [uptime-and-alerts.md](../../runbooks/uptime-and-alerts.md).) And it supplies better evidence than prose: "Google's own Recommender rates this P2, with a HIGH-severity insight behind it" is a stronger line in a submission than "we read our diff --git a/docs/runbooks/status-page.md b/docs/runbooks/status-page.md index 52aa7952..56bc35b6 100644 --- a/docs/runbooks/status-page.md +++ b/docs/runbooks/status-page.md @@ -8,13 +8,12 @@ Public URL: - **Components**: `api.overslash.com` and `app.overslash.com`, each backed by an HTTPS check on 180s cadence across the `eu`/`us`/`as`/`au` regions. - **Monitors** (managed in the Better Stack console, not IaC): - - `api.overslash.com/health` — plain `status` (HTTP up) check. The body also - carries `"db": "up" | "down"` (plus `db_latency_ms` when up), but - `/health` returns **200 even when `db` is `down`** — by design. It backs the - Cloud Run startup *and* liveness probes, so failing it on a Cloud SQL blip - would recycle every container mid-outage and block redeploys until the - database recovered. Use `/ready` for a check that 503s on an unreachable - database; no probe or monitor points at it yet. + - `api.overslash.com/health` — plain `status` (HTTP up) check. `/health` + returns **503 when the database is unreachable** (`"db": "down"`, + `"status": "degraded"`), so this monitor goes red on a Cloud SQL outage as + well as a dead process. It no longer backs the Cloud Run probes, which are + TCP (`infra/modules/cloud-run/main.tf`), so a 503 here never recycles a + container. - `app.overslash.com` — `keyword` check for `_app/immutable`. The dashboard is a SvelteKit SPA shell with no SSR text, so this marker confirms the real app shell is served rather than a Fastly/router error page (e.g. @@ -27,9 +26,11 @@ Public URL: — if GCM itself has an outage the public page stays truthful. The Better Stack monitors mirror the *target* of the GCM uptime check in -`infra/modules/monitoring/uptime.tf`. The GCM `[P0] api_down` alert -(`infra/modules/monitoring/alerts_p0.tf`) continues to page on-call -independently. +`infra/modules/monitoring/uptime.tf`. The GCM `[P0] API Down` alert +(`infra/modules/monitoring/alerts_p0.tf`) is what pages on-call. Until the change +in [uptime-and-alerts.md](uptime-and-alerts.md) is applied, **prod has neither**: +both were `count`-gated on a `domain` that prod leaves empty, so a total prod outage +shows here and pages no one. ## Who has access @@ -65,9 +66,9 @@ is structural. ## On-call -Page is wired only as a status signal. Paging is via the GCM P0 alert pipeline -(PagerDuty integration tracked in TODO.md §1.4, first bullet). When that lands, -link the rotation schedule from this page. +Page is wired only as a status signal. Paging is via the GCM P0 alert pipeline, +which routes to PagerDuty (EU) and email in prod. The API-down half of that +pipeline is [uptime-and-alerts.md](uptime-and-alerts.md). ## Verifying after changes @@ -78,8 +79,9 @@ To sanity-check the monitor without faking an outage: - The Better Stack monitor page shows the last probe time and HTTP response. During an incident, `/health` distinguishes "the process is wedged" (no -response, or a slow one) from "the process is fine but can't reach Postgres" -(`"db":"down"`) without shelling into the container. The body deliberately +response, a slow one, or Cloud Run's own 500 *"the instance could not start +successfully"*) from "the process is fine but can't reach Postgres" (503 with +`"db":"down"`) without shelling into the container. The body deliberately omits the database error — the endpoint is unauthenticated, and a sqlx error names hosts and roles (CASA 6.2.1) — so read the cause from the API's logs (`health probe: database query failed`). The probe is bounded at 2s, so a hung diff --git a/docs/runbooks/uptime-and-alerts.md b/docs/runbooks/uptime-and-alerts.md new file mode 100644 index 00000000..8290fe0f --- /dev/null +++ b/docs/runbooks/uptime-and-alerts.md @@ -0,0 +1,163 @@ +# Uptime check, `[P0] API Down`, and the PromQL alert gates + +Infra is never auto-applied. This page is the human half of the change that gives +production a GCM uptime check and a paging API-down alert, and switches on the three +PromQL alerts that were off in every environment. + +## What changes + +| | dev | prod | +|---|---|---| +| Uptime check `overslash--api-health` on `https:///health` | already exists (`api.dev.overslash.com`) | **created** on `api.overslash.com` | +| `[P0] overslash- API Down` | already exists | **created**, routed to PagerDuty (EU) + email | +| Overview dashboard uptime tile | already wired | **dashboard replaced** so the tile points at the new check | +| `[P1] API Slow Requests` | **created** | **created** | +| `[P1] OAuth Refresh Failure Rate` | **created** | **created** | +| `[P1] Upstream Error Rate` | **created** | **created** | +| `[P1] Webhook Delivery Failure Rate` | query updated in place | query updated in place | +| Actions & OAuth dashboard | replaced (two ratio charts) | replaced (two ratio charts) | + +### Why prod had no uptime check + +`modules/monitoring` creates the uptime check and `[P0] API Down` only when `api_domain` is +non-empty, and the root module fed it `var.domain`. Prod leaves `domain = ""` on purpose: +the GCLB serves `api.overslash.com` and `*.api.overslash.com` (`api_host_suffix`) instead +of a Cloud Run domain mapping. So both resources were silently dropped. The only P0 left +was `API High 5xx Rate`, which cannot fire when the API returns nothing at all. + +`infra/main.tf` now passes `domain`, or `api_host_suffix` when `domain` is empty and the +LB is on. With neither set, nothing public answers, and the check stays off. + +### Why the three PromQL alerts were off, and why they are on now + +Each one is gated (`*_alert_enabled` in tfvars) because GMP rejects a policy whose metric +descriptor does not exist yet. The descriptors were measured present in **both** projects +on 2026-10-27: + +``` +prometheus.googleapis.com/overslash_http_request_duration_seconds/histogram +prometheus.googleapis.com/overslash_oauth_events_total/counter +prometheus.googleapis.com/overslash_upstream_responses_total/counter +``` + +So all three flags are `true` in both tfvars. **But turning on the OAuth and upstream +alerts as they were would have added two alerts that cannot fire.** They divided by +`clamp_min(sum(rate(...)), 1)`. Prod traffic is well under one event per second, so the +denominator was always clamped to 1, and the "ratio" was really failures per second, +which never reaches 0.10 or 0.20. The deployed webhook alert had the same flaw. All three +now copy the latency alert's pattern: a `0.001` floor that only guards against division +by zero, plus an explicit `and` that requires a minimum sample in the window: + +| Alert | Window | Minimum sample | Fires at | +|---|---|---|---| +| OAuth Refresh Failure Rate | 15m | rate > 0.011/s (~10 refreshes) | > 10% failed | +| Upstream Error Rate | 15m | rate > 0.022/s (~20 responses) | > 20% `5xx` / `error` | +| Webhook Delivery Failure Rate | 30m | rate > 0.0056/s (~10 terminal deliveries) | > 5% failed | + +The two matching dashboard charts on *Actions & OAuth* had the same clamp, so they showed +ratios close to zero. They now use the same `0.001` floor. + +### What `/health` reports, and dev at night + +`/health` returns 503 when Postgres is unreachable (D-NEXT in DECISIONS.md), so +`[P0] API Down` covers the API's database as well as the process. Cloud Run's +own startup and liveness probes are TCP probes and never read `/health`, so a +503 never restarts a container. Before that change, an HTTP liveness probe on +`/health` timed out (1 s default, against a 2 s DB check) once the database was +gone, and it shut the instance down about 90 s after each dev stop. + +Dev stops Cloud SQL from 23:00 to 07:00 Europe/Madrid (`infra-scheduler`), so dev's +check fails every night and `[P0] overslash-dev API Down` emails +alert@overspiral.com nightly. That is deliberate: it tests the alert path +end to end. Prod has no scheduler. + +### Relationship to the status page + +[status.overslash.com](status-page.md) (Better Stack) probes the same `/health` URL from +outside GCP. It is a public status signal, managed in the Better Stack console, and it +does not page. Paging comes from this GCM check: it is in IaC, reviewable, and routed +to PagerDuty. Keep both. + +## Apply + +Dev first. That is where GMP gets to reject a malformed PromQL query before prod sees it. + +```bash +make tofu-plan ENV=dev +# expect: 4 to add, 2 to change, 1 to destroy — all under module.monitoring +make tofu-apply ENV=dev + +make tofu-plan ENV=prod +# expect, under module.monitoring only: +# + google_monitoring_uptime_check_config.api_health[0] +# + google_monitoring_alert_policy.api_down[0] +# + google_monitoring_alert_policy.{api_slow_requests,oauth_refresh_failure_rate,upstream_error_rate}[0] +# ~ google_monitoring_alert_policy.webhook_failure_rate[0] +# -/+ google_monitoring_dashboard.{overview,actions_and_oauth} +# ~ terraform_data.dashboard_hash["overview"], ["actions_and_oauth"] +make tofu-apply ENV=prod +``` + +A full prod plan also shows whatever else is pending apply on `dev`. Read the whole plan, +not only the monitoring lines. To apply only this change: +`tofu plan -var-file=env/prod.tfvars -target=module.monitoring -out=prod.tfplan`. + +The agent sandbox could only make a `-refresh=false -target=module.monitoring` plan: the +module's `depends_on` drags in IAM, which the factory SA cannot read, and the plan stops at +the PagerDuty secret read. Up to that point the prod plan matched the list above, minus +`api_down`, whose channels depend on that secret. + +## Verify + +```bash +# 1. The uptime check exists and targets the apex. +gcloud monitoring uptime list-configs --project=overslash \ + --format="table(displayName,monitoredResource.labels.host,httpCheck.path,period)" +# expect: overslash-prod-api-health api.overslash.com /health 60s + +# 2. It passes from every checker region (give it ~5 minutes after apply). +# Console: Monitoring → Uptime checks → overslash-prod-api-health. +# Or the metric itself (replace with the check's ID, the last segment of `name`): +curl -s -G -H "Authorization: Bearer $(gcloud auth print-access-token)" \ + "https://monitoring.googleapis.com/v3/projects/overslash/timeSeries" \ + --data-urlencode 'filter=metric.type="monitoring.googleapis.com/uptime_check/check_passed" AND metric.labels.check_id=""' \ + --data-urlencode "interval.startTime=$(date -u -d '10 min ago' +%FT%TZ)" \ + --data-urlencode "interval.endTime=$(date -u +%FT%TZ)" \ + | jq -r '.timeSeries[] | [.metric.labels.checker_location, .points[0].value.boolValue] | @tsv' +# expect: one row per region, all true + +# 3. The alert policies exist, are enabled, and P0 pages both channels. +curl -s -H "Authorization: Bearer $(gcloud auth print-access-token)" \ + "https://monitoring.googleapis.com/v3/projects/overslash/alertPolicies?pageSize=100" \ + | jq -r '.alertPolicies[] | [.displayName, .enabled, (.notificationChannels|length)] | @tsv' | sort +# expect 14: the 10 already there + [P0] API Down (2 channels) + API Slow Requests, +# OAuth Refresh Failure Rate, Upstream Error Rate + +# 4. The new dashboard tile resolves: Monitoring → Dashboards → overslash-prod Overview, +# the uptime chart shows data instead of "No data". +``` + +`gcloud alpha monitoring policies list` works too, if the alpha component is installed. + +### Test the page without an outage (optional) + +Do this on dev. It pages the dev email channel only: + +```bash +ID=$(gcloud monitoring uptime list-configs --project=overslash-dev \ + --filter='displayName=overslash-dev-api-health' --format='value(name.basename())') +# Point the check at a path that 404s. This drifts from Terraform on purpose. +gcloud monitoring uptime update "$ID" --project=overslash-dev --path=/health-uptime-drill +# Wait ~3-5 minutes (2 failing 60s probes from ≥2 regions); expect +# "[P0] overslash-dev API Down" to open and an email to alert@overspiral.com. +# Revert, either by hand or with an apply, which puts /health back: +gcloud monitoring uptime update "$ID" --project=overslash-dev --path=/health +``` + +There is no safe way to do it in prod short of an outage. The dev run plus check (3) +above, which shows the prod policy wired to PagerDuty, is the evidence. + +## Evidence for CASA + +Record in [gcp-posture.md](../compliance/casa/gcp-posture.md#monitoring--the-count-gate-confirmed): +the output of steps 1 and 3 with the date, and the policy count going from 10 to 14. diff --git a/infra/env/dev.tfvars b/infra/env/dev.tfvars index c66b7c59..72a8a8fc 100644 --- a/infra/env/dev.tfvars +++ b/infra/env/dev.tfvars @@ -148,15 +148,21 @@ terraform_operators = [ # where is the series named against each flag below. Verify before # flipping, one flag at a time. +# All three descriptors were measured present in both projects on 2026-10-27 +# (`.../overslash_http_request_duration_seconds/histogram`, +# `.../overslash_oauth_events_total/counter`, +# `.../overslash_upstream_responses_total/counter`), so all three are on. +# The two ratio queries were fixed in the same change: they clamped a +# per-second denominator to 1, which at our volume made them unable to fire. +# Runbook: docs/runbooks/uptime-and-alerts.md. + # `overslash_http_request_duration_seconds` must be live as a *histogram*, not # the summary it rendered as before the bucket set landed. -api_latency_alert_enabled = false +api_latency_alert_enabled = true -# `overslash_oauth_events_total` and `overslash_upstream_responses_total`. Both -# have most likely been emitted by now; verify the descriptors and flip these on -# in a follow-up. -oauth_refresh_alert_enabled = false -upstream_error_alert_enabled = false +# `overslash_oauth_events_total` and `overslash_upstream_responses_total`. +oauth_refresh_alert_enabled = true +upstream_error_alert_enabled = true # `custom.googleapis.com/overslash/business/integrity_violations` — written by # the metrics exporter, not GMP, but the descriptor rule is the same. gcloud has diff --git a/infra/env/prod.tfvars b/infra/env/prod.tfvars index 38bdfcdf..49c17b1a 100644 --- a/infra/env/prod.tfvars +++ b/infra/env/prod.tfvars @@ -148,15 +148,21 @@ terraform_operators = [ # where is the series named against each flag below. Verify before # flipping, one flag at a time. +# All three descriptors were measured present in both projects on 2026-10-27 +# (`.../overslash_http_request_duration_seconds/histogram`, +# `.../overslash_oauth_events_total/counter`, +# `.../overslash_upstream_responses_total/counter`), so all three are on. +# The two ratio queries were fixed in the same change: they clamped a +# per-second denominator to 1, which at our volume made them unable to fire. +# Runbook: docs/runbooks/uptime-and-alerts.md. + # `overslash_http_request_duration_seconds` must be live as a *histogram*, not # the summary it rendered as before the bucket set landed. -api_latency_alert_enabled = false +api_latency_alert_enabled = true -# `overslash_oauth_events_total` and `overslash_upstream_responses_total`. Both -# have most likely been emitted by now; verify the descriptors and flip these on -# in a follow-up. -oauth_refresh_alert_enabled = false -upstream_error_alert_enabled = false +# `overslash_oauth_events_total` and `overslash_upstream_responses_total`. +oauth_refresh_alert_enabled = true +upstream_error_alert_enabled = true # `custom.googleapis.com/overslash/business/integrity_violations` — written by # the metrics exporter, not GMP, but the descriptor rule is the same. gcloud has diff --git a/infra/main.tf b/infra/main.tf index 1bd697dc..299c47a8 100644 --- a/infra/main.tf +++ b/infra/main.tf @@ -313,11 +313,16 @@ module "monitoring" { upstream_error_alert_enabled = var.upstream_error_alert_enabled api_latency_alert_enabled = var.api_latency_alert_enabled integrity_alert_enabled = var.integrity_alert_enabled - api_domain = var.domain - api_service_name = module.cloud_run.service_name - cloud_sql_instance_name = module.cloud_sql.instance_name - monthly_budget_usd = var.monthly_budget_usd - billing_account_id = var.billing_account_id + # The host the uptime check probes. Prod leaves `domain` empty (the GCLB + # serves `api_host_suffix` and its wildcard instead of a Cloud Run domain + # mapping), which used to leave this empty too and silently drop both the + # uptime check and `[P0] API Down`. Without the LB and without `domain` + # nothing public answers on `api_host_suffix`, so it stays empty there. + api_domain = var.domain != "" ? var.domain : (var.enable_api_lb ? var.api_host_suffix : "") + api_service_name = module.cloud_run.service_name + cloud_sql_instance_name = module.cloud_sql.instance_name + monthly_budget_usd = var.monthly_budget_usd + billing_account_id = var.billing_account_id depends_on = [ google_project_service.apis, diff --git a/infra/modules/cloud-run/main.tf b/infra/modules/cloud-run/main.tf index 06df2aa8..2545f741 100644 --- a/infra/modules/cloud-run/main.tf +++ b/infra/modules/cloud-run/main.tf @@ -694,9 +694,16 @@ resource "google_cloud_run_v2_service" "api" { mount_path = "/cloudsql" } + # TCP, not `/health`: the probes ask only "is the process up", never + # "are its dependencies up". `/health` checks Postgres and 503s when it + # is down, and an HTTP liveness probe on it killed every instance + # within ~90s of a Cloud SQL stop (dev, nightly) — a replacement then + # cannot boot either, so the probe turned a DB outage into an API one. + # The startup probe loses nothing: the listener binds only after + # `create_app` has connected, migrated and preflighted. + # Dependency health is the uptime check's job (modules/monitoring). startup_probe { - http_get { - path = "/health" + tcp_socket { port = var.container_port } initial_delay_seconds = 5 @@ -705,8 +712,7 @@ resource "google_cloud_run_v2_service" "api" { } liveness_probe { - http_get { - path = "/health" + tcp_socket { port = var.container_port } period_seconds = 30 diff --git a/infra/modules/monitoring/alerts_p1.tf b/infra/modules/monitoring/alerts_p1.tf index 14e00c6f..d618cbcb 100644 --- a/infra/modules/monitoring/alerts_p1.tf +++ b/infra/modules/monitoring/alerts_p1.tf @@ -197,11 +197,19 @@ resource "google_monitoring_alert_policy" "oauth_refresh_failure_rate" { display_name = "OAuth refresh failure ratio > 10%" condition_prometheus_query_language { + # The floor is a divide-by-zero guard only (see `api_slow_requests`). A + # `clamp_min(..., 1)` here, as this used to be, pins the denominator at 1 + # whenever traffic is under 1/s — always, in prod — so the "ratio" was + # failures per second and never reached 0.10. The `and` requires ~10 + # refreshes in the window instead. query = <<-PROMQL - sum(rate(overslash_oauth_events_total{flow="refresh",status="failure"}[15m])) - / - clamp_min(sum(rate(overslash_oauth_events_total{flow="refresh"}[15m])), 1) - > 0.10 + ( + sum(rate(overslash_oauth_events_total{flow="refresh",status="failure"}[15m])) + / + clamp_min(sum(rate(overslash_oauth_events_total{flow="refresh"}[15m])), 0.001) + ) > 0.10 + and + sum(rate(overslash_oauth_events_total{flow="refresh"}[15m])) > 0.011 PROMQL duration = "900s" evaluation_interval = "60s" @@ -235,11 +243,16 @@ resource "google_monitoring_alert_policy" "upstream_error_rate" { display_name = "Upstream error ratio > 20%" condition_prometheus_query_language { + # Same floor + sample guard as the OAuth alert above: ~20 upstream + # responses in the window. query = <<-PROMQL - sum(rate(overslash_upstream_responses_total{job="${var.api_service_name}",status_class=~"5xx|error"}[15m])) - / - clamp_min(sum(rate(overslash_upstream_responses_total{job="${var.api_service_name}"}[15m])), 1) - > 0.20 + ( + sum(rate(overslash_upstream_responses_total{job="${var.api_service_name}",status_class=~"5xx|error"}[15m])) + / + clamp_min(sum(rate(overslash_upstream_responses_total{job="${var.api_service_name}"}[15m])), 0.001) + ) > 0.20 + and + sum(rate(overslash_upstream_responses_total{job="${var.api_service_name}"}[15m])) > 0.022 PROMQL duration = "900s" evaluation_interval = "60s" @@ -270,11 +283,16 @@ resource "google_monitoring_alert_policy" "webhook_failure_rate" { display_name = "Webhook terminal failure ratio > 5%" condition_prometheus_query_language { + # Same floor + sample guard as the OAuth alert above: ~10 terminal + # deliveries in the window. query = <<-PROMQL - sum(rate(overslash_webhook_deliveries_total{status="failed",final="true"}[30m])) - / - clamp_min(sum(rate(overslash_webhook_deliveries_total{final="true"}[30m])), 1) - > 0.05 + ( + sum(rate(overslash_webhook_deliveries_total{status="failed",final="true"}[30m])) + / + clamp_min(sum(rate(overslash_webhook_deliveries_total{final="true"}[30m])), 0.001) + ) > 0.05 + and + sum(rate(overslash_webhook_deliveries_total{final="true"}[30m])) > 0.0056 PROMQL duration = "1800s" evaluation_interval = "60s" @@ -322,11 +340,11 @@ resource "google_monitoring_alert_policy" "api_slow_requests" { display_name = "More than 5% of gateway requests slower than 2.5s" condition_prometheus_query_language { - # Note the departure from this module's `clamp_min(..., 1)` idiom: at prod - # volume the denominator is a per-second rate that is legitimately below - # 1, and clamping there would silently deflate the ratio toward zero. The - # floor is only a divide-by-zero guard; the explicit `and` rate guard is - # what enforces a meaningful sample size, legibly. + # Never `clamp_min(..., 1)` a per-second rate: at prod volume the + # denominator is legitimately below 1, and clamping there would silently + # deflate the ratio toward zero. The floor is only a divide-by-zero + # guard; the explicit `and` rate guard is what enforces a meaningful + # sample size, legibly. Every ratio alert in this file follows it. query = <<-PROMQL ( 1 - ( diff --git a/infra/modules/monitoring/dashboards/actions-and-oauth.json.tftpl b/infra/modules/monitoring/dashboards/actions-and-oauth.json.tftpl index 82566260..2c7dc516 100644 --- a/infra/modules/monitoring/dashboards/actions-and-oauth.json.tftpl +++ b/infra/modules/monitoring/dashboards/actions-and-oauth.json.tftpl @@ -56,7 +56,7 @@ "xyChart": { "dataSets": [{ "timeSeriesQuery": { - "prometheusQuery": "sum by (template_key) (rate(overslash_upstream_responses_total{status_class=~\"5xx|error\"}[15m])) / clamp_min(sum by (template_key) (rate(overslash_upstream_responses_total[15m])), 1)" + "prometheusQuery": "sum by (template_key) (rate(overslash_upstream_responses_total{status_class=~\"5xx|error\"}[15m])) / clamp_min(sum by (template_key) (rate(overslash_upstream_responses_total[15m])), 0.001)" }, "plotType": "LINE" }], @@ -87,7 +87,7 @@ "xyChart": { "dataSets": [{ "timeSeriesQuery": { - "prometheusQuery": "sum by (provider) (rate(overslash_oauth_events_total{flow=\"refresh\",status=\"failure\"}[15m])) / clamp_min(sum by (provider) (rate(overslash_oauth_events_total{flow=\"refresh\"}[15m])), 1)" + "prometheusQuery": "sum by (provider) (rate(overslash_oauth_events_total{flow=\"refresh\",status=\"failure\"}[15m])) / clamp_min(sum by (provider) (rate(overslash_oauth_events_total{flow=\"refresh\"}[15m])), 0.001)" }, "plotType": "LINE" }], From f69f7174132bf308fcb13746f80188ce915334e1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:26:08 +0000 Subject: [PATCH 05/10] chore(docs): allocate D141 (#843) Allocated by .github/workflows/allocate-decision.yml. The authoring PR wrote D-NEXT; this commit is the mapping from D141 to the PR that decided it. --- DECISIONS.md | 2 +- SPEC.md | 2 +- TODO.md | 2 +- docs/compliance/casa/gcp-posture.md | 2 +- docs/runbooks/uptime-and-alerts.md | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 0a7eb873..cc9e61b6 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1537,7 +1537,7 @@ The disclosure input parses a form's body as JSON, as it does for a JSON body. **Decision**: For Overslash-managed sign-in (no `org_idp_configs` row claims the provider), login uses the deployment's `{PROVIDER}_AUTH_*` app even when the org has saved OAuth App Credentials (`OAUTH_{PROVIDER}_CLIENT_ID/SECRET` org secrets) for that provider. Those credentials still override env for service connections (SPEC §7). An admin can opt a provider in: a row in `org_oauth_signin_overrides (org_id, provider_key)`, off by default, makes the org's credentials serve sign-in too. It is set from the *Use for sign-in* action on a Google/GitHub row in Org Settings → OAuth App Credentials (`PATCH /v1/org-oauth-credentials/{provider} {override_signin}`), shown only when the org override exists and the env sign-in app is configured, or from the same checkbox on the save form (`PUT … {override_signin}`). Deleting the credentials clears the opt-in. Dedicated IdP rows, including those with `use_org_credentials`, are unchanged: they are already an explicit sign-in choice. There is no backfill, so orgs that had saved Google/GitHub credentials go back to the built-in sign-in app until an admin opts in. `services::org_signin::resolve_org_signin_credentials`; tests in `tests/org_oauth_credentials.rs`. **Rationale**: Admins save org OAuth credentials to brand or scope service connections, and the old precedence quietly moved every member's login onto that app as well. That app may have a narrower consent screen, test-user limits or unverified status, so login could break for reasons unrelated to sign-in. Making the sign-in takeover a separate, visible opt-in keeps "configure connections" from changing "how people log in". -## D-NEXT: `/health` fails when Postgres is down; Cloud Run probes are TCP +## D141: `/health` fails when Postgres is down; Cloud Run probes are TCP **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`. diff --git a/SPEC.md b/SPEC.md index b9978a2a..b03b1558 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1852,7 +1852,7 @@ Every per-IP throttle — these, and the in-handler magic-link, download and upl - **Storage**: Redis/Valkey if available (distributed, accurate across instances); in-memory `DashMap` fallback (single-instance, no external dependency). - **Fail-open**: If Redis becomes unavailable at runtime, requests are allowed through (logged as warning). - **Health endpoint** (`/health`) and **build stamp** (`GET /v1/version`) are exempt from rate limiting. -- **Health vs. liveness**: `/health` is origin health — it runs a bounded `SELECT 1` and returns 200 (`status: ok`) only when Postgres answers, **503** (`status: degraded`) otherwise, reporting `db` (plus `db_latency_ms` when up) in the body; the error itself is logged, never returned, since the endpoint is unauthenticated. It is what the uptime check and the status page probe. `/ready` applies the same gate (`status: ready`). Liveness is not an HTTP endpoint at all: the Cloud Run startup and liveness probes are TCP probes on the container port, so a database outage fails `/health` without restarting containers (D-NEXT). +- **Health vs. liveness**: `/health` is origin health — it runs a bounded `SELECT 1` and returns 200 (`status: ok`) only when Postgres answers, **503** (`status: degraded`) otherwise, reporting `db` (plus `db_latency_ms` when up) in the body; the error itself is logged, never returned, since the endpoint is unauthenticated. It is what the uptime check and the status page probe. `/ready` applies the same gate (`status: ready`). Liveness is not an HTTP endpoint at all: the Cloud Run startup and liveness probes are TCP probes on the container port, so a database outage fails `/health` without restarting containers (D141). - **Build stamp**: both `/health` and `/ready` also report `version` (release, or the crate version for unreleased builds) and `commit` (full git SHA, or `unknown`). `GET /v1/version` returns the same two values plus `commit_short`, without the database probe — it is unauthenticated for the same reason `/health` is, and is what the dashboard renders in its sidebar footer. The SHA is baked in at compile time (`OVERSLASH_GIT_SHA`, supplied by Cloud Build as `$COMMIT_SHA` or discovered via `git` for local builds). --- diff --git a/TODO.md b/TODO.md index 1dc2f424..aa89806e 100644 --- a/TODO.md +++ b/TODO.md @@ -120,7 +120,7 @@ are gaps today. - [ ] Enable container image vulnerability scanning (`containerscanning.googleapis.com`); none of Container Scanning, Container Analysis, Binary Authorization or Security Command Center is enabled. Pairs with the CI-side 6.1.1 work. - [ ] Secret Manager: 16 secrets, none with `rotation`, `next_rotation_time` or `expire_time`. Nothing schedules or alerts on rotation age. (CASA 6.7.1) - [x] **Memorystore has neither AUTH nor transit encryption** — `overslash-prod-valkey` confirmed live: `authEnabled` unset, `transitEncryptionMode: DISABLED`, `BASIC` tier. **In code, pending apply:** `infra/modules/memorystore/main.tf` sets `auth_enabled = true` and `transit_encryption_mode = "SERVER_AUTHENTICATION"`, and publishes the AUTH string and server CA to Secret Manager (`overslash--valkey-auth`, `-valkey-server-ca`), which reach the API (`REDIS_PASSWORD`, `REDIS_CA_CERT`) and oversla-sh (`VALKEY_*`) by `secret_key_ref`. Both clients connect `rediss://` and verify the server against only that CA (`services/valkey.rs`, `oversla-sh/src/storage.rs`). **Operator step still open:** TLS forces a replacement of the instance (downtime, empty keyspace, oversla.sh links lost unless exported) — pre-flight, apply and verify per [memorystore-auth-tls.md](docs/runbooks/memorystore-auth-tls.md). (CASA 6.7.1) -- [x] **Production has 0 uptime checks and no `[P0] API Down` alert** — 8 alert policies were deployed (10 on 2026-10-27), which was exactly 12 declared minus 3 disabled in tfvars minus the API-down policy `count`-gated on `api_domain != ""`. The only deployed P0 is `API High 5xx Rate`, which cannot fire when the service returns nothing at all. Root cause: `infra/main.tf` fed the gate `var.domain`, which prod leaves empty because the GCLB serves `api_host_suffix` instead of a domain mapping. It now falls back to `api_host_suffix` when the LB is on, which creates `overslash-prod-api-health` on `api.overslash.com/health` and `[P0] API Down` (PagerDuty + email). The 3 disabled gates (`api_latency`, `oauth_refresh`, `upstream_error`) are on in both envs: their descriptors were measured present on 2026-10-27. The OAuth, upstream and already-deployed webhook ratios clamped a per-second denominator to 1, so at prod volume they could never fire. They now use a `0.001` floor + a minimum-sample guard, like the latency alert. Better Stack (status.overslash.com) still probes the same URL, but it is console-managed and does not page. **Applied 2026-10-27, both envs**, verified per [uptime-and-alerts.md](docs/runbooks/uptime-and-alerts.md): prod has 14 policies (`[P0] API Down` on PagerDuty + email) and `overslash-prod-api-health` passing from all 6 checker regions. Found on the way: dev's `/health` 500'd every night, because the HTTP liveness probe on `/health` (1 s default timeout against a 2 s DB check) killed the instance about 90 s after the Cloud SQL stop, and no replacement could boot. Cloud Run probes are now TCP, and `/health` returns 503 when Postgres is down (D-NEXT), so the prod page covers the database too. **That part is in code, pending a Cloud Run apply + an API deploy.** +- [x] **Production has 0 uptime checks and no `[P0] API Down` alert** — 8 alert policies were deployed (10 on 2026-10-27), which was exactly 12 declared minus 3 disabled in tfvars minus the API-down policy `count`-gated on `api_domain != ""`. The only deployed P0 is `API High 5xx Rate`, which cannot fire when the service returns nothing at all. Root cause: `infra/main.tf` fed the gate `var.domain`, which prod leaves empty because the GCLB serves `api_host_suffix` instead of a domain mapping. It now falls back to `api_host_suffix` when the LB is on, which creates `overslash-prod-api-health` on `api.overslash.com/health` and `[P0] API Down` (PagerDuty + email). The 3 disabled gates (`api_latency`, `oauth_refresh`, `upstream_error`) are on in both envs: their descriptors were measured present on 2026-10-27. The OAuth, upstream and already-deployed webhook ratios clamped a per-second denominator to 1, so at prod volume they could never fire. They now use a `0.001` floor + a minimum-sample guard, like the latency alert. Better Stack (status.overslash.com) still probes the same URL, but it is console-managed and does not page. **Applied 2026-10-27, both envs**, verified per [uptime-and-alerts.md](docs/runbooks/uptime-and-alerts.md): prod has 14 policies (`[P0] API Down` on PagerDuty + email) and `overslash-prod-api-health` passing from all 6 checker regions. Found on the way: dev's `/health` 500'd every night, because the HTTP liveness probe on `/health` (1 s default timeout against a 2 s DB check) killed the instance about 90 s after the Cloud SQL stop, and no replacement could boot. Cloud Run probes are now TCP, and `/health` returns 503 when Postgres is down (D141), so the prod page covers the database too. **That part is in code, pending a Cloud Run apply + an API deploy.** - [x] **No SSL policy and no Cloud Armor on the production LB** — both `gcloud compute ssl-policies list` and `security-policies list` are empty. TLS 1.2/1.3-only is currently true but unpinned and uncitable. `infra/modules/api-lb/main.tf` now declares `overslash-prod-api-ssl-policy` (TLS 1.2 minimum, `RESTRICTED`) on the HTTPS proxy and `overslash-prod-api-armor` on the backend service — default allow, OWASP CRS rule sets in preview (logged, never blocked) until tuned. **Not applied** — in code, pending a human run of [lb-hardening.md](docs/runbooks/lb-hardening.md), which also lists the evidence commands. (CASA 4.1.1) - [x] Set the org-policy baseline that would make the above guardrails rather than conventions. **Applied 2026-10-27** in both projects. The overspiral.com org now enforces a baseline at org level (measured table in [gcp-posture.md](docs/compliance/casa/gcp-posture.md#supply-chain-and-organization-policy)), and `infra/modules/project-guardrails/` ([project-guardrails.md](docs/runbooks/project-guardrails.md)) pins the Overslash-specific ones per project: `sql.managed.restrictPublicIp` + `restrictAuthorizedNetworks` (enforced in prod, explicitly off on dev's public-IP database), `storage.publicAccessPrevention` and `iam.disableAuditLoggingExemption` (both projects). `iam.managed.disableServiceAccountKeyCreation` is inherited from the org, and dev's hand-set opt-out for the Directory-sync key is adopted into Terraform. Org-wide policies belong to the org repo, not here. - [ ] **`run.allowedIngress`** — unset (allow all) on both projects, measured 2026-10-27. It applies project-wide, so it can only be set to `internal-and-cloud-load-balancing` once overfwd and the shortener stop running with `INGRESS_TRAFFIC_ALL` (prod API is already LB-only). (CASA 6.2.1) diff --git a/docs/compliance/casa/gcp-posture.md b/docs/compliance/casa/gcp-posture.md index 22343126..2e130384 100644 --- a/docs/compliance/casa/gcp-posture.md +++ b/docs/compliance/casa/gcp-posture.md @@ -335,7 +335,7 @@ Re-measured after the apply (2026-10-27, prod): Follow-up, in code, pending a Cloud Run apply and an API deploy: `/health` returns 503 when Postgres is down, and the Cloud Run probes are TCP, so `[P0] API Down` also covers -the database (D-NEXT in DECISIONS.md). +the database (D141 in DECISIONS.md). ### Storage — production diff --git a/docs/runbooks/uptime-and-alerts.md b/docs/runbooks/uptime-and-alerts.md index 8290fe0f..351dbaff 100644 --- a/docs/runbooks/uptime-and-alerts.md +++ b/docs/runbooks/uptime-and-alerts.md @@ -59,7 +59,7 @@ ratios close to zero. They now use the same `0.001` floor. ### What `/health` reports, and dev at night -`/health` returns 503 when Postgres is unreachable (D-NEXT in DECISIONS.md), so +`/health` returns 503 when Postgres is unreachable (D141 in DECISIONS.md), so `[P0] API Down` covers the API's database as well as the process. Cloud Run's own startup and liveness probes are TCP probes and never read `/health`, so a 503 never restarts a container. Before that change, an HTTP liveness probe on From eb94af661410a15af82d2e6b3085d88ac2b2c2b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81ngel=20Manuel=20Mart=C3=ADn?= Date: Wed, 28 Oct 2026 13:11:31 +0100 Subject: [PATCH 06/10] feat(dashboard): single Allow split button with duration menu on approval detail (#844) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(dashboard): single Allow split button with duration menu on approval detail Folds "Allow once", "Allow & Remember" and the expiry clock into one primary split button: the caret picks Once / remember For 1h…30d / Always, the main half runs it. Once resolves `allow`; any other choice resolves `allow_remember` at the "Remember as" scope with that TTL. Co-Authored-By: Claude Opus 5.5 * fix(dashboard): screenshot-approvals waits for load, not networkidle Authenticated pages hold an SSE stream open, so networkidle never fires and goto times out — the same reason snap.mjs dropped it. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Factory Co-authored-by: Claude Opus 5.5 --- UI_SPEC.md | 15 +- dashboard/scripts/screenshot-approvals.mjs | 20 +- .../src/lib/components/ApprovalDetail.svelte | 39 ++- .../approval/AllowSplit.stories.svelte | 21 ++ .../lib/components/approval/AllowSplit.svelte | 229 ++++++++++++++++++ 5 files changed, 283 insertions(+), 41 deletions(-) create mode 100644 dashboard/src/lib/components/approval/AllowSplit.stories.svelte create mode 100644 dashboard/src/lib/components/approval/AllowSplit.svelte diff --git a/UI_SPEC.md b/UI_SPEC.md index cf299114..d00ab800 100644 --- a/UI_SPEC.md +++ b/UI_SPEC.md @@ -290,11 +290,11 @@ A unified search component used across Services, Template Catalog, Audit Log, Ap ### Split Button -A button with two halves joined seamlessly — left side is the default action, right side is a dropdown to choose an alternative. Used for **Allow & Remember** (approval resolution) and **Refresh Control**. +A button with two halves joined seamlessly — left side is the default action, right side is a dropdown to choose an alternative. Used for **Allow** (approval resolution) and **Refresh Control**. **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 = "Allow & Remember" (applies most specific tier). Right dropdown = specificity picker showing all tiers with radio selection + expiry dropdown. +**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. ### Notifications Dropdown @@ -492,12 +492,13 @@ ALL permission keys for a request must be covered for auto-approval. A single mi ### Approval resolution -When a user resolves an approval (from the detail panel, notification dropdown, or standalone approval page), two actions are presented: +When a user resolves an approval (from the detail panel, notification dropdown, or standalone approval page), the action bar presents: -- **Allow & Remember** — a **split button** (see Design System). Left side applies the most specific tier (safest default). Right dropdown (▾) opens the specificity picker to choose a broader scope. +- **Remember as** — the specificity picker (below). The most specific tier is pre-selected. - **Deny** +- **Allow** — a **split button** (see Design System). Left side runs the selected duration (default: once). Right dropdown (▾) picks Once, or remember For 1 hour … 30 days / Always at the scope chosen in "Remember as". -#### Specificity picker (split button dropdown) +#### Specificity picker ("Remember as" dropdown) The dropdown reads `suggested_tiers` from the approval API. Each tier includes a `description` (English label generated by Overslash). The permission key string is shown as secondary text. @@ -514,12 +515,10 @@ The dropdown reads `suggested_tiers` from the approval API. Each tier includes a │ │ │ ○ Any GitHub action │ │ github:*:* │ -│ │ -│ Expires: [24h ▾] │ └──────────────────────────────────────────────────────┘ ``` -The most specific tier is pre-selected. Selecting a tier and clicking "Allow & Remember" (or just clicking the left side of the split button) stores permission keys at that scope. +The most specific tier is pre-selected. Allowing with any remember duration (For … / Always) stores permission keys at the selected scope; "Allow once" stores nothing. For `http` service approvals, tiers compose paired keys: diff --git a/dashboard/scripts/screenshot-approvals.mjs b/dashboard/scripts/screenshot-approvals.mjs index a1153f87..af459ce0 100644 --- a/dashboard/scripts/screenshot-approvals.mjs +++ b/dashboard/scripts/screenshot-approvals.mjs @@ -44,7 +44,7 @@ try { const ctx = await browser.newContext({ viewport: { width: 1280, height: 800 } }); const page = await ctx.newPage(); await page.goto(`${session.dashboardUrl}/approvals/${approval.id}`, { - waitUntil: 'networkidle' + waitUntil: 'load' }); await page.waitForURL(/\/login\?return_to=/, { timeout: 10_000 }); await page.waitForTimeout(500); @@ -115,9 +115,9 @@ try { } }); - // 7. The merged action bar's two dropdowns, open. Picking a tier here is - // what "Allow & Remember" writes — the scope ladder and the expiry menu - // live in the bar, not in a side panel. + // 7. The merged action bar's two dropdowns, open. The scope ladder is what + // a remembering allow writes; the Allow split button's menu picks once + // vs. remember-for- — both live in the bar, not in a side panel. { const { page: p, ctx } = await snap.navigateAndSnap( 'detail-remember-open', @@ -134,10 +134,14 @@ try { } ); await p.keyboard.press('Escape'); - await p.getByRole('button', { name: /Rule expires/ }).click(); - await p.getByRole('listbox', { name: /Rule expiry/i }).waitFor({ timeout: 5_000 }); + // The Allow split button's caret picks once vs. remember-for-. + await p.getByRole('button', { name: 'More allow options' }).click(); + await p.getByRole('menu', { name: /Allow duration/i }).waitFor({ timeout: 5_000 }); await p.waitForTimeout(300); - await snap.snap(p, 'detail-expiry-open'); + await snap.snap(p, 'detail-allow-open'); + await p.getByRole('menuitemradio', { name: /For 24 hours/ }).click(); + await p.getByRole('button', { name: 'Allow for 24 hours', exact: true }).waitFor({ timeout: 5_000 }); + await snap.snap(p, 'detail-allow-24h'); await ctx.close(); } @@ -155,7 +159,7 @@ try { body: '{"n":2}' }); const { page: p, ctx } = await snap.page({ viewport: { width: 1280, height: 800 } }); - await p.goto(`${session.dashboardUrl}/approvals`, { waitUntil: 'networkidle' }); + await p.goto(`${session.dashboardUrl}/approvals`, { waitUntil: 'load' }); await p.getByRole('button', { name: 'Approve once', exact: true }).first().waitFor({ timeout: 15_000 }); await p.getByRole('button', { name: 'Approve once', exact: true }).first().click(); await p.getByRole('status').waitFor({ timeout: 10_000 }); diff --git a/dashboard/src/lib/components/ApprovalDetail.svelte b/dashboard/src/lib/components/ApprovalDetail.svelte index 60aa1c36..cd2dd031 100644 --- a/dashboard/src/lib/components/ApprovalDetail.svelte +++ b/dashboard/src/lib/components/ApprovalDetail.svelte @@ -10,7 +10,7 @@ import RiskBadge from './approval/RiskBadge.svelte'; import ServiceTile from './approval/ServiceTile.svelte'; import RememberControl from './approval/RememberControl.svelte'; - import ExpiryControl from './approval/ExpiryControl.svelte'; + import AllowSplit from './approval/AllowSplit.svelte'; import { relativeTime } from '#lib/utils/time.ts'; import { shortEmail } from '#lib/identityDisplay.ts'; import { createResolution } from '#lib/approvals/resolution.svelte.ts'; @@ -63,7 +63,8 @@ let selectedTier = $state(0); let useCustomKey = $state(false); let customKey = $state(''); - let ttl = $state('forever'); + // 'once' or a TTL_OPTIONS value — what the Allow split button runs. + let dur = $state('once'); let formError = $state(null); const submitting = $derived(ctrl.submitting); const error = $derived(formError ?? ctrl.error); @@ -91,7 +92,7 @@ selectedTier = 0; useCustomKey = false; customKey = ''; - ttl = 'forever'; + dur = 'once'; formError = null; }); @@ -174,13 +175,17 @@ return; } body.remember_keys = keys; - if (ttl !== 'forever') body.ttl = ttl; + if (dur !== 'forever') body.ttl = dur; } const updated = await ctrl.resolve(body); if (!updated) return; pushToast('success', resolutionToast(resolution, before, updated, body.remember_keys)); onResolved?.(updated); } + + function allow(d: string) { + void resolve(d === 'once' ? 'allow' : 'allow_remember'); + }

@@ -217,8 +222,8 @@
{#if isPending} - +
-
- - +
{#if willRunInBackground} @@ -673,9 +668,6 @@ .ovs-btn-danger:not(:disabled):hover { background: var(--badge-bg-danger); } - .aq-ab-primary { - font-weight: 500; - } /* status bars (non-pending states) */ .aq-statusbar { @@ -1117,15 +1109,12 @@ align-items: stretch; gap: 10px; } - /* stack the merged bar: scope, then expiry, then the buttons */ + /* stack the merged bar: scope, then the buttons */ .aq-actionbar :global(.aq-remember) { order: 1; } - .aq-actionbar :global(.aq-expctl) { - order: 2; - } .aq-ab-btns { - order: 3; + order: 2; } .aq-ab-btns .ovs-btn { flex: 1; diff --git a/dashboard/src/lib/components/approval/AllowSplit.stories.svelte b/dashboard/src/lib/components/approval/AllowSplit.stories.svelte new file mode 100644 index 00000000..e0e632a7 --- /dev/null +++ b/dashboard/src/lib/components/approval/AllowSplit.stories.svelte @@ -0,0 +1,21 @@ + + + + + + diff --git a/dashboard/src/lib/components/approval/AllowSplit.svelte b/dashboard/src/lib/components/approval/AllowSplit.svelte new file mode 100644 index 00000000..e052f6ae --- /dev/null +++ b/dashboard/src/lib/components/approval/AllowSplit.svelte @@ -0,0 +1,229 @@ + + +
+ + + + {#if open} + + {/if} +
+ + From 489e684d30a2d6ebdfa66af1fd674bf945c512e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81ngel=20Manuel=20Mart=C3=ADn?= Date: Wed, 28 Oct 2026 14:16:24 +0100 Subject: [PATCH 07/10] feat(approvals): org-configurable default Allow & Remember expiry (7 days) (#846) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(approvals): org-configurable default Allow & Remember expiry (7 days) Orgs get default_remember_ttl_secs (migration 139; NULL = forever, 60s..365d, default 7 days, backfilled to existing orgs), exposed on /v1/orgs/{id}/execution-settings and as a select in Org Settings. An allow_remember resolve with no ttl now takes the org default instead of minting a permanent rule; "forever" is accepted explicitly. The dashboard expiry picker and the MCP remember dialog preselect the org default, and the dashboard, MCP and SDK card send "forever" explicitly. One parser (parse_rule_ttl) now serves /resolve and permission edits. Co-Authored-By: Claude Opus 5.5 * fix(dashboard): space before the Allow menu's "· default" tag Svelte trimmed the leading space inside the tag span, so it read "For 7 days· default". Screenshot script waits out the menu fade-in. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Factory Co-authored-by: Claude Opus 5.5 --- ...e6056319d5d685559a97f5d43e02ec73d8e5.json} | 16 +- ...954e0ab2f4c556a857feb98adae6c78f5192c.json | 28 +++ ...babd9b87734dc6b5798d6a5493599b83be5e.json} | 5 +- DECISIONS.md | 6 + SCHEMA.sql | 13 +- SPEC.md | 4 +- UI_SPEC.md | 9 +- .../src/routes/approvals/resolve.rs | 25 ++- .../src/routes/mcp/elicitation.rs | 110 ++++++++-- .../src/routes/mcp/initialize.rs | 2 +- crates/overslash-api/src/routes/mcp/modern.rs | 38 +++- .../src/routes/mcp/tools_call.rs | 1 + crates/overslash-api/src/routes/orgs/mod.rs | 4 + .../overslash-api/src/routes/orgs/settings.rs | 27 ++- .../overslash-api/src/routes/permissions.rs | 18 +- .../overslash-api/src/services/mcp_session.rs | 26 ++- .../tests/approvals_remember_keys.rs | 200 ++++++++++++++++++ crates/overslash-api/tests/mcp_elicitation.rs | 12 +- crates/overslash-core/src/types/duration.rs | 40 ++++ .../139_org_default_remember_ttl.down.sql | 3 + .../139_org_default_remember_ttl.up.sql | 25 +++ crates/overslash-db/src/repos/org.rs | 35 ++- .../screenshot-remember-ttl-default.mjs | 72 +++++++ dashboard/src/lib/approvals/format.ts | 42 +++- .../src/lib/components/ApprovalDetail.svelte | 27 ++- .../lib/components/approval/AllowSplit.svelte | 31 ++- .../components/approval/ExpiryControl.svelte | 4 +- .../components/settings/OrgSettings.svelte | 43 ++++ dashboard/src/lib/types.ts | 5 + dashboard/tests/scenarios/index.mjs | 1 + dashboard/tests/scenarios/seed.mjs | 14 ++ sdk/src/elements/approval-card.ts | 6 +- sdk/src/format/index.ts | 6 + sdk/src/types/approvals.ts | 5 +- sdk/test/elements.test.ts | 19 ++ 35 files changed, 832 insertions(+), 90 deletions(-) rename .sqlx/{query-1dd2e60c7b39789db4fd126784dccc7a27080a5f7dea5d42af0afc77b77877db.json => query-0390276a145cf74b5d8e3e2f85b2e6056319d5d685559a97f5d43e02ec73d8e5.json} (72%) create mode 100644 .sqlx/query-73d37304206f85ae5a92dbdadaf954e0ab2f4c556a857feb98adae6c78f5192c.json rename .sqlx/{query-260a1ab8de1ea4997f5802ca889105e22ac0c8da565d0f7941f498d66d7262fd.json => query-e3412d889cff77af86f7441ec2abbabd9b87734dc6b5798d6a5493599b83be5e.json} (65%) create mode 100644 crates/overslash-db/migrations/139_org_default_remember_ttl.down.sql create mode 100644 crates/overslash-db/migrations/139_org_default_remember_ttl.up.sql create mode 100644 dashboard/scripts/screenshot-remember-ttl-default.mjs 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 11eb0686..82b241c8 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 00000000..67c63c90 --- /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 cca348da..8cf4e6a4 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 cc9e61b6..28f6b3bd 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 c0a00679..a1e9a9a2 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 b03b1558..c22eb6c8 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 d00ab800..b64cd143 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 4e789ea2..5791c2de 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 da0d8860..f0c8f982 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 da8ed1bc..1d32afac 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 2d5315f5..981f6276 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 e2bd27b8..5b81943a 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 8a0808bc..d629ab02 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 4f5139a2..5ffe763f 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 65f4a793..f0626560 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 35507fd0..cfb7f681 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 9db538fe..98c48ec2 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 4a994ede..336f50df 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 d1bf9e9f..28502567 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 00000000..bfe62da3 --- /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 00000000..37a20057 --- /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 2c83eab0..62e478f7 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 00000000..81e98f20 --- /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 7b819616..76b3a445 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 cd2dd031..34cd20c7 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 e052f6ae..aaf88616 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 @@