From 4d7bd3deebf35f87aece5aab933dbe39570ef53e Mon Sep 17 00:00:00 2001 From: calvin-archastro Date: Fri, 23 Oct 2026 10:31:52 -0700 Subject: [PATCH] feat(aster-srv): person tokens read every dashboard dataset; route-table auth gate - Person tokens (scope read, cache:read still accepted) read every dataset the /ui pages show over /v1 JSON, for any served repository named with ?repo=, and are refused every write. New GET /v1/test-results/clusters reuses the dashboard's query. - aster srv get|cache|flaky|broken|quarantine|clusters|test prints that JSON with the stored sign-in (CI credentials win when present). - routes::ROUTES is the single route table (method, pattern, auth class, handler). The router authenticates from headers before reading any body, deny by default for /v1; lookup/upload bodies are capped at 1 MiB. - Writers: cache writes need event_name push; repository_owner_id pins; writer workflows required when writers are configured; pull_request_target refused for results uploads. Exact IAP audience; nbf; generic token errors; stale JWKS keys evicted in both verifiers; JWKS fetch shared in iap.rs; person lookups do not count; blob stand-in behind ASTER_SRV_DEV_BLOB_ROUTE. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 3 + crates/aster-srv/README.md | 31 +- crates/aster-srv/src/audit.rs | 27 +- crates/aster-srv/src/auth.rs | 57 +- crates/aster-srv/src/cache/mod.rs | 12 +- crates/aster-srv/src/cache/policy.rs | 26 +- crates/aster-srv/src/cache/routes.rs | 44 +- crates/aster-srv/src/config.rs | 129 +++- crates/aster-srv/src/http.rs | 256 ++++--- crates/aster-srv/src/iap.rs | 167 ++++- crates/aster-srv/src/lib.rs | 1 + crates/aster-srv/src/person.rs | 22 +- crates/aster-srv/src/routes.rs | 370 +++++++++ .../aster-srv/src/test_results/durations.rs | 1 + crates/aster-srv/src/test_results/query.rs | 47 +- crates/aster-srv/src/ui/mod.rs | 13 +- crates/aster-srv/tests/cache.rs | 4 +- crates/aster-srv/tests/cli_auth.rs | 218 +++++- crates/aster-srv/tests/common/mod.rs | 11 +- .../tests/fixtures/test-other-key.pem | 27 + crates/aster-srv/tests/hardening.rs | 25 +- crates/aster-srv/tests/query.rs | 21 +- crates/aster-srv/tests/routes.rs | 699 ++++++++++++++++++ docs/test-tracking.md | 29 +- src/cache/mod.rs | 4 +- src/cli/commands.rs | 69 ++ src/cli/mod.rs | 2 +- src/main.rs | 39 +- src/test_results/client.rs | 36 + src/test_results/mod.rs | 1 + src/test_results/read.rs | 174 +++++ tests/cache_login_e2e.rs | 334 +++++++-- tests/cache_remote_e2e.rs | 1 + 33 files changed, 2544 insertions(+), 356 deletions(-) create mode 100644 crates/aster-srv/src/routes.rs create mode 100644 crates/aster-srv/tests/fixtures/test-other-key.pem create mode 100644 crates/aster-srv/tests/routes.rs create mode 100644 src/test_results/read.rs diff --git a/README.md b/README.md index 5c615ebb..844dbf2f 100644 --- a/README.md +++ b/README.md @@ -310,6 +310,9 @@ your company account and confirm, and Aster stores a read-only token under out, Aster prints one line (`not signed in; run aster cache login`) and continues with the local cache. See [Reading the cache from a laptop](docs/test-tracking.md#reading-the-cache-from-a-laptop). +The same sign-in reads the flaky, broken, cluster and quarantine data the +dashboard shows: `aster srv flaky`, `aster srv clusters`, `aster srv get ` +print JSON ([details](docs/test-tracking.md#reading-the-dashboards-data)). A hit restores declared `outputs` from the artifact cache and skips execution. `aster cache status` reads that store's per-project index. `--json` prints diff --git a/crates/aster-srv/README.md b/crates/aster-srv/README.md index 6d97e2ba..2ae832da 100644 --- a/crates/aster-srv/README.md +++ b/crates/aster-srv/README.md @@ -400,7 +400,7 @@ Pages are sent with a strict `Content-Security-Policy`, `nosniff`, | `ASTER_SRV_BROKEN_RUNS` | `3` | Consecutive default-branch runs failing the same way that make a test broken; at least 3. | | `ASTER_SRV_MAX_BODY_BYTES` | 64 MiB | Largest upload, before and after decompression. | | `ASTER_SRV_RAW_BUCKET` | unset | A GCS bucket that keeps each upload as sent, using the pod's Workload Identity. Give the bucket a lifecycle rule that deletes objects after the retention period. | -| `ASTER_SRV_IAP_AUDIENCE` | unset | The Google IAP audience the dashboard accepts, exact or a prefix ending in `/`. Unset turns the dashboard off. See [Dashboard](#dashboard). | +| `ASTER_SRV_IAP_AUDIENCE` | unset | The Google IAP audience the dashboard accepts: exactly `/projects/NUMBER/global/backendServices/ID`. A project-wide prefix is refused at startup, since it would accept an assertion for any other backend service of the project. Unset turns the dashboard off. See [Dashboard](#dashboard). | | `ASTER_SRV_UI_EMAIL_DOMAINS` | unset | Comma-separated domains (`example.com`, `*.example.com`) whose emails may view the dashboard. Unset allows anyone who passes IAP. | | `ASTER_SRV_UI_DEV_EMAIL` | unset | The viewer to trust when no IAP audience is set. Development only. | | `ASTER_SRV_QUARANTINE_RUNS` | `2` | Different runs with a definite flake in 7 days that quarantine a test; `0` turns automatic quarantine off. | @@ -412,7 +412,9 @@ Pages are sent with a strict `Content-Security-Policy`, `nosniff`, | `ASTER_SRV_CACHE_SIGNER` | unset | The pod's Workload Identity service account email. Required with `ASTER_SRV_CACHE_BUCKET`; blob URLs are signed as this account through `signBlob`. | | `ASTER_SRV_CACHE_DAYS` | `14` | Drop manifests unused this long, then blobs no remaining manifest names, and uncommitted uploads older than this. | | `ASTER_SRV_CACHE_WRITERS` | `*=refs/heads/` | Comma-separated `owner/name=ref` or `*=ref` patterns that may upload and commit. A pattern ending in `*` matches that prefix. The pattern is compared with the token's `ref` claim. | -| `ASTER_SRV_CACHE_WRITER_WORKFLOWS` | unset | Comma-separated prefixes of the token's `job_workflow_ref` claim (`owner/repo/.github/workflows/ci.yml@`; each must contain `@`). When set, a CI token writes the cache or uploads test results only from a matching workflow, in addition to the ref rule. Unset applies no workflow limit, so existing deployments keep working until it is set. Static tokens are exempt. | +| `ASTER_SRV_CACHE_WRITER_WORKFLOWS` | unset | Comma-separated prefixes of the token's `job_workflow_ref` claim (`owner/repo/.github/workflows/ci.yml@`; each must contain `@`). When set, a CI token writes the cache or uploads test results only from a matching workflow, in addition to the ref rule. Required whenever `ASTER_SRV_CACHE_WRITERS` is set (startup fails otherwise unless `ASTER_SRV_ALLOW_ANY_WRITER_WORKFLOW=1`, a development escape hatch). Static tokens are exempt. | +| `ASTER_SRV_REPOSITORY_OWNER_IDS` | unset | Comma-separated `owner/name=id` or `owner/*=id`. A token for a matching repository must carry that `repository_owner_id` claim, so a repository recreated under the same name by someone else is refused with `403`. | +| `ASTER_SRV_DEV_BLOB_ROUTE` | unset | `1` makes the in-memory blob store answer `PUT\|GET //cas/`, the test stand-in for a signed GCS URL. Tests and local development only; no deploy sets it and the route does not exist without it. | | `ASTER_SRV_PUBLIC_URL` | unset | The https URL people reach the server at (`https://aster-srv.example.com`); the `iss` and `aud` of person tokens. Required for [Laptop sign-in](#laptop-sign-in). With an https URL every response carries `Strict-Transport-Security: max-age=31536000; includeSubDomains`. An `http://` URL is refused at startup unless `ASTER_SRV_ALLOW_HTTP_PUBLIC_URL=1` (development and tests only). | | `ASTER_SRV_CLI_TOKEN_KEYS` | unset | `kid:secret` pairs, comma-separated. A secret is hex (64+ digits) or base64 (43+ characters) for at least 32 random bytes, as `openssl rand -base64 32` makes; passphrases, short keys and patterned keys (one byte repeated, a repeated block, a counting run, fewer than 16 distinct bytes) stop startup. The first signs person tokens and form MACs; all verify. Setting it also requires `ASTER_SRV_IAP_AUDIENCE` and `ASTER_SRV_UI_EMAIL_DOMAINS`, and forbids `ASTER_SRV_UI_DEV_EMAIL`. Keep it in a secret store. | | `ASTER_SRV_CLI_TOKEN_TTL` | `43200` | Seconds a person token lives, 60 to 43200 (12 hours; verification enforces the same cap). | @@ -565,8 +567,27 @@ Writes accept a static development token, or a CI token whose `ref` matches `refs/heads/main`) and, when `ASTER_SRV_CACHE_WRITER_WORKFLOWS` is set, whose `job_workflow_ref` (else `workflow_ref`) starts with one of its prefixes. A pull-request ref is refused with `403`, so a branch cannot poison the cache, -and so is a person's token. Test-result uploads follow the same workflow -rule; a person's token is refused on every test-result route. +and so is a person's token. Cache writes also need `event_name` `push` +(pull requests, `pull_request_target`, `workflow_run`, merge queues, schedules +and manual dispatches are `403` whatever ref they carry). Test-result uploads +follow the workflow rule and refuse `pull_request_target`, but accept pull +requests, since a retry that passes there is the flake signal. + +### Route table and who may call what + +`routes::ROUTES` is the one list of routes. Each has an auth class and the +router checks it, from the headers, before it reads a body or runs a handler; +a `/v1/` path the table does not list is `401` without a valid token. + +| Class | Routes | Who | +|---|---|---| +| `Public` | `GET /healthz`, `GET /readyz`, `POST /v1/auth/token` | anyone; the exchange needs a valid code | +| `Iap` | `/cli-auth`, `/ui/*` | a person behind IAP | +| `CiOrPerson` | `GET /v1/test-results/{tests,tests/:id,failures,quarantine,clusters,durations,runs/:id/failures}`, `GET /v1/cache/{summary,entries}`, `POST /v1/cache/lookup` | a CI token for its own repository, or a person's token for any served repository named with `?repo=` (read-only; a person's lookups do not count hits) | +| `CiWriter` | `POST /v1/test-results/runs`, `POST /v1/cache/{upload,commit}` | a CI token only, then the writer rules | + +A lookup or upload request body is capped at 1 MiB and read only after the +token is verified; a commit may be as large as `ASTER_SRV_MAX_BODY_BYTES`. ## Laptop sign-in @@ -582,7 +603,7 @@ an authorization-code flow with PKCE against the CLI's loopback port. A person's token is an HS256 JWT signed by this server: `iss` is `ASTER_SRV_PUBLIC_URL`, `aud` is the same URL, `sub` is IAP's stable subject (`accounts.google.com:`), `email` is the email IAP verified, `scope` is -`cache:read`, with `iat`, `nbf`, `exp` (at most 12 hours after `iat`) and +`read` (it covers the cache and every dashboard dataset), with `iat`, `nbf`, `exp` (at most 12 hours after `iat`) and `jti`, and a `kid` header. Verification pins HS256, requires every claim, and allows 30 seconds of clock skew. Tokens issued before `sub`, `nbf` and `email` were required are refused; those people sign in once more. There diff --git a/crates/aster-srv/src/audit.rs b/crates/aster-srv/src/audit.rs index dc6ce95b..e7d75aa5 100644 --- a/crates/aster-srv/src/audit.rs +++ b/crates/aster-srv/src/audit.rs @@ -11,7 +11,7 @@ //! | `auth_failure` | any 401 or 403 | `subject` if known, `reason`, `detail` | //! | `rate_limited` | a limit refused the request | `scope`, `subject` if known | //! -//! Each carries `route` (a fixed template, see [`route_label`]) and +//! Each carries `route` (a fixed template, see [`crate::routes::label`]) and //! `client_ip` from the request. Tokens, codes, verifiers and assertions are //! never fields: callers pass reasons and subjects only, and [`crate::log`] //! redacts token-shaped text as a second line of defence. @@ -68,31 +68,6 @@ pub fn was_audited() -> bool { .unwrap_or(false) } -/// The templated route of a request, a bounded set safe as a metric label. -pub fn route_label(method: &hyper::Method, path: &str) -> &'static str { - use crate::test_results::query; - match path { - "/healthz" => "/healthz", - "/readyz" => "/readyz", - "/metrics" => "/metrics", - "/cli-auth" => "/cli-auth", - "/v1/healthz" => "/v1/healthz", - "/v1/auth/token" => "/v1/auth/token", - "/v1/test-results/runs" => "/v1/test-results/runs", - "/v1/test-results/durations" => "/v1/test-results/durations", - p if p == "/ui" || p.starts_with("/ui/") => "/ui", - p if p.starts_with("/v1/test-results/") && query::parse_route(p).is_some() => { - "/v1/test-results/query" - } - p if p.starts_with("/v1/cache/") => "/v1/cache", - p if p.starts_with("/v1/") => "/v1/other", - _ => { - let _ = method; - "other" - } - } -} - fn emit(event: &str, route: Option<&'static str>, fields: Value) { let (ip, route) = CTX .try_with(|ctx| (Some(ctx.client_ip.clone()), Some(ctx.route))) diff --git a/crates/aster-srv/src/auth.rs b/crates/aster-srv/src/auth.rs index 7f014472..b65cef97 100644 --- a/crates/aster-srv/src/auth.rs +++ b/crates/aster-srv/src/auth.rs @@ -20,7 +20,7 @@ use jsonwebtoken::{decode, decode_header, Algorithm, DecodingKey, Validation}; use serde::Deserialize; use tokio::sync::{Mutex, RwLock}; -use crate::config::{Config, RepositoryAllowlist, StaticToken}; +use crate::config::{Config, OwnerIds, RepositoryAllowlist, StaticToken}; use crate::person::{Person, TokenIssuer}; /// Who a bearer token names. @@ -45,7 +45,8 @@ pub async fn authenticate( if verifier.is_static(token) { return verifier.verify(authorization).await.map(Principal::Ci); } - let header = decode_header(token).map_err(|e| AuthError::Invalid(e.to_string()))?; + let header = + decode_header(token).map_err(|_| AuthError::Invalid("the token is malformed".into()))?; match header.alg { Algorithm::HS256 => match tokens { Some(tokens) => tokens.verify(token).map(Principal::Person), @@ -91,6 +92,10 @@ pub struct Identity { /// `workflow_ref` when the claim is absent. Writer workflow prefixes /// match it. `None` for a static token. pub job_workflow: Option, + /// The `event_name` claim (`push`, `pull_request`, `workflow_dispatch`, + /// `merge_group`...): what triggered the run. Cache writes need `push`. + /// `None` for a static token. + pub event_name: Option, /// A configured static token, not an OIDC token. pub static_token: bool, } @@ -119,12 +124,18 @@ impl AuthError { /// The reply to send: 403 for a valid token that may not use the /// server, 401 for everything else. + /// + /// The reply says only that the token is not valid. The reason, which + /// never contains the token, goes to the log. pub fn into_reply(self) -> crate::Reply { let status = if matches!(self, AuthError::Forbidden(_)) { 403 } else { 401 }; + if let AuthError::Invalid(reason) = &self { + crate::log::warn("token refused", serde_json::json!({"reason": reason})); + } crate::Reply::error(status, self.to_string()) } } @@ -133,7 +144,7 @@ impl std::fmt::Display for AuthError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { AuthError::Missing => write!(f, "send a bearer token"), - AuthError::Invalid(reason) => write!(f, "the token is not valid: {reason}"), + AuthError::Invalid(_) => write!(f, "the token is not valid"), AuthError::Forbidden(repository) => { write!(f, "{repository} may not use this server") } @@ -156,6 +167,10 @@ struct Claims { workflow: Option, #[serde(default)] job_workflow_ref: Option, + #[serde(default)] + event_name: Option, + #[serde(default, deserialize_with = "string_or_number")] + repository_owner_id: Option, } /// GitHub sends `run_id` as a string; accept a number too rather than @@ -182,6 +197,7 @@ pub struct Verifier { audience: String, issuer: String, allowlist: RepositoryAllowlist, + owner_ids: OwnerIds, static_tokens: Vec, /// Where keys come from; `None` uses only keys added with [`Self::with_key`]. jwks_url: Option, @@ -198,6 +214,7 @@ impl Verifier { audience: config.audience.clone(), issuer: config.issuer.clone(), allowlist: config.repositories.clone(), + owner_ids: config.owner_ids.clone(), static_tokens: config.static_tokens.clone(), jwks_url: Some(format!( "{}/.well-known/jwks", @@ -243,11 +260,13 @@ impl Verifier { sha: None, workflow: None, job_workflow: None, + event_name: None, static_token: true, }); } - let header = decode_header(token).map_err(|e| AuthError::Invalid(e.to_string()))?; + let header = decode_header(token) + .map_err(|_| AuthError::Invalid("the token is malformed".into()))?; if header.alg != Algorithm::RS256 { return Err(AuthError::Invalid( "tokens must be signed with RS256".into(), @@ -262,10 +281,16 @@ impl Verifier { validation.set_audience(&[&self.audience]); validation.set_issuer(&[&self.issuer]); validation.set_required_spec_claims(&["exp", "aud", "iss"]); + validation.validate_nbf = true; + validation.leeway = 5; let claims = decode::(token, &key, &validation) - .map_err(|e| AuthError::Invalid(e.to_string()))? + .map_err(|e| AuthError::Invalid(format!("verification failed: {:?}", e.kind())))? .claims; - if !self.allowlist.allows(&claims.repository) { + if !self.allowlist.allows(&claims.repository) + || !self + .owner_ids + .accepts(&claims.repository, claims.repository_owner_id.as_deref()) + { return Err(AuthError::Forbidden(claims.repository)); } let workflow = non_empty(claims.workflow); @@ -276,6 +301,7 @@ impl Verifier { sha: non_empty(claims.sha), job_workflow: non_empty(claims.job_workflow_ref).or_else(|| workflow.clone()), workflow, + event_name: non_empty(claims.event_name), static_token: false, }) } @@ -308,12 +334,16 @@ impl Verifier { .json() .await .map_err(|e| AuthError::Invalid(format!("cannot read signing keys: {e}")))?; - let mut keys = self.keys.write().await; + // The fetched set replaces the old one: the map stays as small as the + // issuer's set, and a key the issuer withdrew stops verifying. + let mut fetched = HashMap::new(); for jwk in &set.keys { if let (Some(id), Ok(key)) = (&jwk.common.key_id, DecodingKey::from_jwk(jwk)) { - keys.insert(id.clone(), key); + fetched.insert(id.clone(), key); } } + let mut keys = self.keys.write().await; + *keys = fetched; keys.get(kid) .cloned() .ok_or_else(|| AuthError::Invalid("unknown signing key".to_string())) @@ -502,7 +532,7 @@ mod tests { "ASTER_SRV_REPOSITORIES" => Some("ArchAstro/*".into()), "ASTER_SRV_STATIC_TOKENS" => Some("ArchAstro/dev=dev-token-0123456789".into()), "ASTER_SRV_PUBLIC_URL" => Some("https://aster-srv.example.com".into()), - "ASTER_SRV_IAP_AUDIENCE" => Some("/projects/1/global/backendServices/".into()), + "ASTER_SRV_IAP_AUDIENCE" => Some("/projects/1/global/backendServices/4242".into()), "ASTER_SRV_UI_EMAIL_DOMAINS" => Some("example.com".into()), "ASTER_SRV_CLI_TOKEN_KEYS" => Some(keys.clone()), _ => None, @@ -752,6 +782,11 @@ mod tests { let mut verifier = Verifier::new(&config()); verifier.jwks_url = Some(url); + // A key the issuer no longer lists, held from an earlier fetch. + verifier + .keys + .get_mut() + .insert("withdrawn".into(), DecodingKey::from_secret(b"x")); let verifier = Arc::new(verifier); let bearer = format!("Bearer {}", token(claims("ArchAstro/firstlanding"), "slow")); let mut calls = tokio::task::JoinSet::new(); @@ -771,6 +806,10 @@ mod tests { 1, "one fetch serves them all" ); + assert!( + !verifier.keys.read().await.contains_key("withdrawn"), + "a refetch drops keys the issuer withdrew" + ); // A key the issuer really does not have still fails, and does not refetch within a minute. let unknown = format!("Bearer {}", token(claims("ArchAstro/firstlanding"), "nope")); diff --git a/crates/aster-srv/src/cache/mod.rs b/crates/aster-srv/src/cache/mod.rs index 4fd407de..87987c6f 100644 --- a/crates/aster-srv/src/cache/mod.rs +++ b/crates/aster-srv/src/cache/mod.rs @@ -20,7 +20,17 @@ mod store; use crate::db::Migration; -pub use policy::workflow_allowed; +pub use policy::{results_writer_allowed, workflow_allowed}; + +/// Whether a signed-in person may read `repo`'s data: their email domain is +/// allowed and the server serves the repository. +pub fn person_may_browse( + app: &crate::App, + person: &crate::person::Person, + repo: &str, +) -> Result<(), &'static str> { + policy::person_browser(&app.iap, person, repo, &app.config.repositories) +} pub use routes::{handle, Headers}; pub use signing::{expires_at, FixedSigner, Signer, URL_TTL}; pub use store::{run_retention, spawn, BlobStore, Report}; diff --git a/crates/aster-srv/src/cache/policy.rs b/crates/aster-srv/src/cache/policy.rs index 9ccc73bd..d8357a88 100644 --- a/crates/aster-srv/src/cache/policy.rs +++ b/crates/aster-srv/src/cache/policy.rs @@ -76,8 +76,27 @@ pub fn workflow_allowed(config: &Config, identity: &Identity) -> bool { }) } +/// Whether the run was triggered by a push. Pull requests (also from forks +/// and `pull_request_target`), merge queues and manual dispatches are not +/// trusted to write the cache, whatever ref they carry. +fn pushed(identity: &Identity) -> bool { + identity.static_token || identity.event_name.as_deref() == Some("push") +} + +/// Whether a CI token may upload test results. Results come from pull +/// requests too (a retry that passes there is the flake signal), so unlike +/// cache writes this does not require a push; it refuses only +/// `pull_request_target`, which runs with the base repository's secrets +/// against the pull request's code, and a workflow outside the configured +/// writer workflows. +pub fn results_writer_allowed(config: &Config, identity: &Identity) -> bool { + workflow_allowed(config, identity) + && (identity.static_token || identity.event_name.as_deref() != Some("pull_request_target")) +} + /// A CI caller. Static tokens may write; OIDC tokens write only when their -/// ref matches the writer patterns and their workflow is allowed. +/// ref matches the writer patterns, their workflow is allowed and the run +/// was triggered by a push. pub fn ci_reader(config: &Config, identity: &Identity) -> Reader { Reader { repository: identity.repository.clone(), @@ -85,7 +104,8 @@ pub fn ci_reader(config: &Config, identity: &Identity) -> Reader { || (config .cache_writers .allows(&identity.repository, identity.git_ref.as_deref()) - && workflow_allowed(config, identity)), + && workflow_allowed(config, identity) + && pushed(identity)), writer: Writer::from_identity(identity), caller: Caller::Ci, } @@ -158,6 +178,7 @@ mod tests { "ASTER_SRV_DATABASE_URL" => Some("postgres://localhost/x".into()), "ASTER_SRV_REPOSITORIES" => Some("ArchAstro/*".into()), "ASTER_SRV_UI_EMAIL_DOMAINS" => Some("example.com".into()), + "ASTER_SRV_ALLOW_ANY_WRITER_WORKFLOW" => Some("1".into()), other => extra .iter() .find(|(key, _)| *key == other) @@ -176,6 +197,7 @@ mod tests { job_workflow: Some( "ArchAstro/firstlanding/.github/workflows/ci.yml@refs/heads/main".into(), ), + event_name: Some("push".into()), static_token: false, } } diff --git a/crates/aster-srv/src/cache/routes.rs b/crates/aster-srv/src/cache/routes.rs index 3b5020eb..fe8fffe1 100644 --- a/crates/aster-srv/src/cache/routes.rs +++ b/crates/aster-srv/src/cache/routes.rs @@ -60,7 +60,7 @@ pub async fn handle( // end-to-end test rewrites that host to this server and keeps the signed // path, so `PUT //cas/` stores the bytes commit later hashes. // A GCS-backed server does not answer this path at all. - if app.cache_store.is_memory() { + if app.config.dev_blob_route && app.cache_store.is_memory() { if let Some(digest) = test_blob_digest(path) { return test_blob(app, method, digest, body).await; } @@ -146,14 +146,19 @@ async fn lookup(app: &App, headers: &Headers<'_>, body: Value) -> Reply { if let Err(reply) = same_repo(&reader, &request.repo) { return reply; } - match lookup_hits(app, &reader.repository, &request.keys).await { + // Only CI's lookups count and keep entries alive; a person reading must + // not be able to hold an entry in the cache by asking for it. + let counts = reader.caller == Caller::Ci; + match lookup_hits(app, &reader.repository, &request.keys, counts).await { Ok(hits) => { let requested = request .keys .iter() .collect::>() .len(); - record_lookup(app, &reader.repository, hits.len(), requested); + if counts { + record_lookup(app, &reader.repository, hits.len(), requested); + } Reply::json(200, json!({"hits": hits})) } Err(error) => db_error(error), @@ -217,17 +222,18 @@ async fn lookup_hits( app: &App, repo: &str, keys: &[String], + counts: bool, ) -> anyhow::Result> { let client = app.pool.get().await?; - let rows = client - .query( - "UPDATE cache.manifests \ - SET last_used_at = now(), hits = hits + 1, last_hit_at = now() \ - WHERE repo = $1 AND key = ANY($2) \ - RETURNING key, manifest", - &[&repo, &keys], - ) - .await?; + let sql = if counts { + "UPDATE cache.manifests \ + SET last_used_at = now(), hits = hits + 1, last_hit_at = now() \ + WHERE repo = $1 AND key = ANY($2) \ + RETURNING key, manifest" + } else { + "SELECT key, manifest FROM cache.manifests WHERE repo = $1 AND key = ANY($2)" + }; + let rows = client.query(sql, &[&repo, &keys]).await?; let mut hits = serde_json::Map::new(); for row in rows { let key: String = row.get(0); @@ -237,12 +243,14 @@ async fn lookup_hits( if !app.cache_store.exists(&digest).await? { continue; } - client - .execute( - "UPDATE cache.blobs SET last_used_at = now() WHERE sha256 = $1", - &[&digest], - ) - .await?; + if counts { + client + .execute( + "UPDATE cache.blobs SET last_used_at = now() WHERE sha256 = $1", + &[&digest], + ) + .await?; + } blobs.insert( digest.clone(), Value::String(app.cache_store.signed_url(&digest, Method::Get).await?), diff --git a/crates/aster-srv/src/config.rs b/crates/aster-srv/src/config.rs index 782de709..8db88287 100644 --- a/crates/aster-srv/src/config.rs +++ b/crates/aster-srv/src/config.rs @@ -9,8 +9,8 @@ use anyhow::{bail, Context, Result}; pub const GITHUB_ISSUER: &str = "https://token.actions.githubusercontent.com"; /// A Google IAP audience, `/projects/NUMBER/global/backendServices/ID`, or -/// with the ID left off (a trailing `/`) to stand for any backend service of -/// the project. Both NUMBER and ID are nonempty runs of digits. +/// with the ID left off (a trailing `/`), which parses but is not accepted +/// as a configured audience. Both NUMBER and ID are nonempty runs of digits. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct IapAudience<'a> { pub project: &'a str, @@ -73,10 +73,10 @@ pub struct Config { /// or not. Default 64 MiB. pub max_body_bytes: usize, /// `ASTER_SRV_IAP_AUDIENCE`: the Google IAP audience the dashboard (`/ui`) - /// accepts, `/projects/NUMBER/global/backendServices/ID` (or, for GKE - /// ingress, which assigns the ID, a prefix ending in `/` such as - /// `/projects/NUMBER/global/backendServices/`). Unset disables the - /// dashboard unless `ASTER_SRV_UI_DEV_EMAIL` is set. + /// accepts: exactly `/projects/NUMBER/global/backendServices/ID`. A + /// project-wide prefix is refused at startup, since it would accept an + /// assertion for any other backend service of the project. Unset + /// disables the dashboard unless `ASTER_SRV_UI_DEV_EMAIL` is set. pub iap_audience: Option, /// `ASTER_SRV_UI_EMAIL_DOMAINS`: comma-separated domains (`example.com`, /// or `*.example.com` for subdomains) whose emails may view the @@ -109,6 +109,16 @@ pub struct Config { /// token may write the cache or upload test results only from a job /// whose workflow matches one. Unset applies no workflow restriction. pub writer_workflows: Option, + /// `ASTER_SRV_REPOSITORY_OWNER_IDS`: comma-separated `owner/name=id` or + /// `owner/*=id`. A token for a matching repository must carry that + /// `repository_owner_id` claim, so a repository recreated under the same + /// name by someone else is refused. Unset pins nothing. + pub owner_ids: OwnerIds, + /// `ASTER_SRV_DEV_BLOB_ROUTE=1`: answer `PUT|GET //cas/` + /// from the in-memory blob store, the stand-in for a signed GCS URL. + /// For tests and local development only; unset in every deploy, which + /// uses GCS and has no such route. + pub dev_blob_route: bool, /// `ASTER_SRV_PUBLIC_URL`: the URL people reach this server at, the /// `iss` of the tokens `aster cache login` hands out. Unset turns /// person login off. @@ -268,13 +278,16 @@ impl Config { }; let iap_audience = var("ASTER_SRV_IAP_AUDIENCE").map(|audience| audience.trim().to_string()); + // Exactly one backend service: a prefix would accept an assertion IAP + // issued for another backend service of the project. if iap_audience .as_deref() - .is_some_and(|a| IapAudience::parse(a).is_none()) + .is_none_or(|a| IapAudience::parse(a).is_none_or(|a| a.backend_service.is_none())) + && iap_audience.is_some() { bail!( - "ASTER_SRV_IAP_AUDIENCE must look like /projects/NUMBER/global/backendServices/ID \ - (or end in / to match any backend service of a project)" + "ASTER_SRV_IAP_AUDIENCE must be the full backend-service audience, \ + /projects/NUMBER/global/backendServices/ID" ); } let number = |name: &str, default: u32, min: u32, max: u32| -> Result { @@ -327,6 +340,18 @@ impl Config { None => None, }; let allow_http_public_url = var("ASTER_SRV_ALLOW_HTTP_PUBLIC_URL").as_deref() == Some("1"); + // Naming who may write without naming the workflow lets any workflow + // in the repository on that ref write. Say so, or opt out loudly. + if var("ASTER_SRV_CACHE_WRITERS").is_some() + && writer_workflows.is_none() + && var("ASTER_SRV_ALLOW_ANY_WRITER_WORKFLOW").as_deref() != Some("1") + { + bail!( + "ASTER_SRV_CACHE_WRITERS needs ASTER_SRV_CACHE_WRITER_WORKFLOWS (the workflows \ + that may write); set ASTER_SRV_ALLOW_ANY_WRITER_WORKFLOW=1 only in development" + ); + } + let owner_ids = OwnerIds::parse(var("ASTER_SRV_REPOSITORY_OWNER_IDS").as_deref())?; let public_url = match var("ASTER_SRV_PUBLIC_URL") { Some(url) => { let url = url.trim().trim_end_matches('/').to_string(); @@ -447,6 +472,8 @@ impl Config { cache_days, cache_writers, writer_workflows, + owner_ids, + dev_blob_route: var("ASTER_SRV_DEV_BLOB_ROUTE").as_deref() == Some("1"), public_url, cli_token_keys, cli_token_ttl, @@ -629,6 +656,52 @@ impl RepositoryAllowlist { } } +/// `repository_owner_id` pins: `owner/name` or `owner/*` to a GitHub id. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct OwnerIds(Vec<(String, String)>); + +impl OwnerIds { + pub fn parse(value: Option<&str>) -> Result { + let mut pins = Vec::new(); + for entry in value + .unwrap_or("") + .split(',') + .map(str::trim) + .filter(|e| !e.is_empty()) + { + let (repo, id) = entry.split_once('=').with_context(|| { + format!("ASTER_SRV_REPOSITORY_OWNER_IDS entry '{entry}' must be repo=id") + })?; + let repo = repo.trim().to_ascii_lowercase(); + let id = id.trim(); + if repo + .split_once('/') + .is_none_or(|(o, n)| o.is_empty() || n.is_empty()) + || id.is_empty() + || !id.bytes().all(|b| b.is_ascii_digit()) + { + bail!("ASTER_SRV_REPOSITORY_OWNER_IDS entry '{entry}' must be owner/name=digits or owner/*=digits"); + } + pins.push((repo, id.to_string())); + } + Ok(Self(pins)) + } + + /// Whether `repository` with this `repository_owner_id` claim is + /// acceptable: no pin covers it, or the claim equals the pin. + pub fn accepts(&self, repository: &str, claim: Option<&str>) -> bool { + let repository = repository.to_ascii_lowercase(); + let owner = repository.split_once('/').map(|(owner, _)| owner); + self.0 + .iter() + .filter(|(pattern, _)| { + *pattern == repository + || pattern.strip_suffix("/*").is_some_and(|o| Some(o) == owner) + }) + .all(|(_, id)| claim == Some(id.as_str())) + } +} + /// `repo=ref` patterns that may write the cache. `*` matches any repository. /// A ref pattern matches exactly, or as a prefix when it ends in `*`. #[derive(Debug, Clone, PartialEq, Eq)] @@ -793,7 +866,7 @@ mod tests { let config = with(&[ ( "ASTER_SRV_IAP_AUDIENCE", - "/projects/1/global/backendServices/", + "/projects/1/global/backendServices/4242", ), ("ASTER_SRV_UI_EMAIL_DOMAINS", "example.com"), ("ASTER_SRV_PUBLIC_URL", "https://aster-srv.example.com/"), @@ -859,7 +932,7 @@ mod tests { ("ASTER_SRV_PUBLIC_URL", "https://aster-srv.example.com"), ( "ASTER_SRV_IAP_AUDIENCE", - "/projects/1/global/backendServices/", + "/projects/1/global/backendServices/4242", ), ("ASTER_SRV_UI_EMAIL_DOMAINS", "example.com"), ( @@ -1063,14 +1136,14 @@ mod tests { let mut pairs = REQUIRED.to_vec(); pairs.push(( "ASTER_SRV_IAP_AUDIENCE", - "/projects/123456/global/backendServices/", + "/projects/123456/global/backendServices/987", )); pairs.push(("ASTER_SRV_UI_EMAIL_DOMAINS", "Example.com, *.corp.example")); pairs.push(("ASTER_SRV_UI_DEV_EMAIL", "me@example.com")); let config = Config::from_env(env_of(&pairs)).unwrap(); assert_eq!( config.iap_audience.as_deref(), - Some("/projects/123456/global/backendServices/") + Some("/projects/123456/global/backendServices/987") ); let domains = config.ui_email_domains.unwrap(); assert!(domains.allows("a@example.com")); @@ -1078,6 +1151,11 @@ mod tests { for (name, bad) in [ ("ASTER_SRV_IAP_AUDIENCE", "no-slash"), + // The project-wide prefix would accept another backend's assertion. + ( + "ASTER_SRV_IAP_AUDIENCE", + "/projects/123456/global/backendServices/", + ), ("ASTER_SRV_IAP_AUDIENCE", "/"), ("ASTER_SRV_IAP_AUDIENCE", "/projects/"), ("ASTER_SRV_IAP_AUDIENCE", "/projects/123456/"), @@ -1102,6 +1180,31 @@ mod tests { } } + #[test] + fn writers_need_workflows_and_owner_ids_pin_repositories() { + let mut pairs = REQUIRED.to_vec(); + pairs.push(("ASTER_SRV_CACHE_WRITERS", "*=refs/heads/main")); + assert!(Config::from_env(env_of(&pairs)).is_err()); + pairs.push(("ASTER_SRV_ALLOW_ANY_WRITER_WORKFLOW", "1")); + assert!(Config::from_env(env_of(&pairs)).is_ok()); + let mut pairs = REQUIRED.to_vec(); + pairs.push(("ASTER_SRV_CACHE_WRITERS", "*=refs/heads/main")); + pairs.push(( + "ASTER_SRV_CACHE_WRITER_WORKFLOWS", + "ArchAstro/firstlanding/.github/workflows/ci.yml@", + )); + assert!(Config::from_env(env_of(&pairs)).is_ok()); + + let pins = OwnerIds::parse(Some("ArchAstro/*=123, other/x=9")).unwrap(); + assert!(pins.accepts("archastro/firstlanding", Some("123"))); + assert!(!pins.accepts("ArchAstro/firstlanding", Some("124"))); + assert!(!pins.accepts("ArchAstro/firstlanding", None)); + assert!(pins.accepts("unpinned/repo", None)); + for bad in ["a/b", "a/b=x", "ab=1", "=1", "a/*="] { + assert!(OwnerIds::parse(Some(bad)).is_err(), "{bad}"); + } + } + #[test] fn email_domains_match_exactly_or_by_wildcard() { let domains = EmailDomains::parse("example.com, *.gserviceaccount.com").unwrap(); diff --git a/crates/aster-srv/src/http.rs b/crates/aster-srv/src/http.rs index 577b1939..16fb5fcb 100644 --- a/crates/aster-srv/src/http.rs +++ b/crates/aster-srv/src/http.rs @@ -24,10 +24,15 @@ use tokio::sync::Semaphore; use crate::audit::{self, RequestCtx}; use crate::auth::{Identity, Principal}; use crate::limits; +use crate::routes::{self, Handler}; use crate::test_results::ingest::{self, Upload}; use crate::test_results::{durations, query}; use crate::App; +/// The most a cache lookup or upload request may send after it is +/// authenticated: a list of up to 1,024 digests is about 100 KiB. +const SMALL_BODY_BYTES: usize = 1 << 20; + /// A response before it is sent. #[derive(Debug)] pub struct Reply { @@ -98,7 +103,7 @@ impl Reply { } /// `429` with how many seconds to wait. - fn too_many(seconds: u64) -> Self { + pub(crate) fn too_many(seconds: u64) -> Self { Self::error(429, "too many requests; slow down") .with_header("retry-after", limits::retry_after(seconds)) } @@ -277,7 +282,7 @@ async fn handle( ); let path = request.uri().path().to_string(); let ctx = RequestCtx::new( - audit::route_label(request.method(), &path), + routes::label(request.method().as_str(), &path), client.to_string(), Arc::clone(&app.metrics), ); @@ -301,13 +306,21 @@ async fn handle( } /// What the request body may weigh on this route; 0 means it takes none. -fn body_limit(app: &App, method: &Method, path: &str) -> usize { - match (method.as_str(), path) { - ("POST", "/cli-auth" | "/v1/auth/token") => crate::cli_auth::MAX_FORM_BYTES, - ("POST", "/v1/test-results/runs") => app.config.max_body_bytes, - ("POST" | "PUT", path) if path.starts_with("/v1/cache/") => app.config.max_body_bytes, - // The in-memory blob store's rewritten signed URL. - ("PUT", path) if path.split('/').nth(2) == Some("cas") => app.config.max_body_bytes, +/// Each handler names its own cap, so a route added to the table with a +/// handler that reads a body must state one here. +fn body_limit(app: &App, route: Option<&routes::Route>, path: &str) -> usize { + let Some(route) = route else { return 0 }; + match route.handler { + Handler::CliAuthAuthorize | Handler::TokenExchange => crate::cli_auth::MAX_FORM_BYTES, + Handler::ResultsUpload => app.config.max_body_bytes, + // A lookup or upload request is a list of keys or digests; only a + // commit carries a manifest. + Handler::CacheLookup | Handler::CacheWrite if path != "/v1/cache/commit" => { + SMALL_BODY_BYTES.min(app.config.max_body_bytes) + } + Handler::CacheWrite => app.config.max_body_bytes, + // The in-memory blob store's rewritten signed URL takes an upload. + Handler::SignedBlob if route.method == "PUT" => app.config.max_body_bytes, _ => 0, } } @@ -322,7 +335,13 @@ async fn guarded( let is_get = request.method() == Method::GET; if surface == Surface::Api { // Only /v1/*. Checked before anything else, including readiness. - if !path.starts_with("/v1/") { + // The router and the auth gate are the main surface's, unchanged: a + // `/v1/` route is gated the same on both listeners. A route that + // relies on IAP, which this listener has none of, is not served. + if !path.starts_with("/v1/") + || routes::lookup(request.method().as_str(), &path) + .is_some_and(|route| route.auth == routes::Auth::Iap) + { return Reply::error(404, "not found"); } // A bare liveness answer for the load balancer's health check: it @@ -358,14 +377,18 @@ async fn guarded( } } // Refuse an oversized or unexpected body before reading any of it. - let limit = body_limit(app, request.method(), &path); + let matched = routes::lookup(request.method().as_str(), &path); + let limit = body_limit(app, matched, &path); let declared = request .headers() .get(CONTENT_LENGTH) .and_then(|value| value.to_str().ok()) .and_then(|value| value.parse::().ok()); let chunked = request.headers().contains_key(TRANSFER_ENCODING); - if declared.is_some_and(|length| length > limit) || (limit == 0 && chunked) { + // A known path with the wrong method is a 405 whatever it declares: no + // handler runs and no body is read. + let wrong_method = matched.is_none() && routes::path_known(&path); + if !wrong_method && (declared.is_some_and(|length| length > limit) || (limit == 0 && chunked)) { return Reply::error( 413, if limit == 0 { @@ -378,44 +401,6 @@ async fn guarded( route(app, request).await } -/// Whether `method path` is a `/v1` route that authenticates a bearer token. -fn is_v1_authed(method: &Method, path: &str) -> bool { - match (method.as_str(), path) { - ("POST", "/v1/test-results/runs") | ("GET", "/v1/test-results/durations") => true, - ("GET", "/v1/cache/summary" | "/v1/cache/entries") => true, - ("POST", path) if path.starts_with("/v1/cache/") => true, - ("GET", path) => query::parse_route(path).is_some(), - _ => false, - } -} - -/// Authenticate once, up front, on every `/v1` route that takes a token: a -/// bad token is audited and counted here whichever handler it was for, and a -/// good one is charged to its subject's rate limit. Handlers verify again -/// for their own policy; the check is a signature, not a query. -async fn preauth(app: &App, request: &Request) -> Result<(), Reply> { - let authorization = header(request, AUTHORIZATION); - let principal = match app.principal(authorization.as_deref()).await { - Ok(principal) => principal, - Err(error) => { - let subject = match &error { - crate::auth::AuthError::Forbidden(repository) => Some(repository.as_str()), - _ => None, - }; - audit::auth_failure(subject, error.reason_code(), &error.to_string()); - return Err(error.into_reply()); - } - }; - let subject = match &principal { - Principal::Ci(identity) => format!("ci:{}", identity.repository), - Principal::Person(person) => format!("person:{}", person.subject), - }; - app.limits.api_subject.check(&subject).map_err(|seconds| { - audit::rate_limited("subject", Some(&subject)); - Reply::too_many(seconds) - }) -} - async fn route(app: &App, request: Request) -> Reply { // Until the migrations are applied the tables may not match what the // handlers query. `/healthz` stays up so the process is not killed while @@ -429,15 +414,29 @@ async fn route(app: &App, request: Request) -> Reply { } return Reply::json(503, body); } - if is_v1_authed(request.method(), request.uri().path()) { - if let Err(reply) = preauth(app, &request).await { - return reply; - } - } let path = request.uri().path().to_string(); - match (request.method(), path.as_str()) { - (&Method::GET, "/healthz") => Reply::json(200, json!({"ok": true})), - (&Method::GET, "/readyz") => match ready(app).await { + let method = request.method().as_str().to_string(); + let route = routes::lookup(&method, &path); + // Authenticate before anything else runs or any body is read. + let principal = if routes::needs_gate(route, &path) { + let authorization = header(&request, AUTHORIZATION); + match routes::gate(app, route, authorization.as_deref()).await { + Ok(principal) => principal, + Err(reply) => return reply, + } + } else { + None + }; + let Some(route) = route else { + return if routes::path_known(&path) { + Reply::error(405, "method not allowed") + } else { + Reply::error(404, "not found") + }; + }; + match route.handler { + Handler::Healthz => Reply::json(200, json!({"ok": true})), + Handler::Readyz => match ready(app).await { Ok(()) => Reply::json(200, json!({"ok": true})), Err(error) => { // The error can carry connection and schema details; this @@ -449,11 +448,11 @@ async fn route(app: &App, request: Request) -> Reply { Reply::error(503, "the database is unavailable") } }, - (&Method::GET, "/cli-auth") => { + Handler::CliAuthConfirm => { let assertion = header_name(&request, crate::iap::IAP_HEADER); crate::cli_auth::confirm(app, assertion.as_deref(), request.uri().query()).await } - (&Method::POST, "/cli-auth") => { + Handler::CliAuthAuthorize => { let assertion = header_name(&request, crate::iap::IAP_HEADER); let body = match read_body( request, @@ -467,7 +466,7 @@ async fn route(app: &App, request: Request) -> Reply { }; crate::cli_auth::authorize(app, assertion.as_deref(), &body).await } - (&Method::POST, "/v1/auth/token") => { + Handler::TokenExchange => { let body = match read_body( request, crate::cli_auth::MAX_FORM_BYTES, @@ -487,13 +486,12 @@ async fn route(app: &App, request: Request) -> Reply { }; crate::cli_auth::exchange(app, &body).await } - (&Method::POST, "/v1/test-results/runs") => { - let identity = match authenticate(app, &request).await { - Ok(identity) => identity, - Err(reply) => return reply, + Handler::ResultsUpload => { + let Some(Principal::Ci(identity)) = principal else { + return Reply::error(403, "a person's token is read-only"); }; // Uploads, like cache writes, may be limited to named workflows. - if !crate::cache::workflow_allowed(&app.config, &identity) { + if !crate::cache::results_writer_allowed(&app.config, &identity) { return Reply::error(403, "this workflow may not upload test results"); } let gzip = @@ -506,79 +504,85 @@ async fn route(app: &App, request: Request) -> Reply { }; ingest::handle(app, &identity, Upload { body, gzip }).await } - (&Method::GET, "/v1/test-results/durations") => { - let identity = match authenticate(app, &request).await { + Handler::Durations => { + let identity = match read_identity(app, principal, &request) { Ok(identity) => identity, Err(reply) => return reply, }; durations::handle(app, &identity, request.uri().query()).await } - (&Method::GET, path) if query::parse_route(path).is_some() => { - let identity = match authenticate(app, &request).await { + Handler::ResultsRead => { + let identity = match read_identity(app, principal, &request) { Ok(identity) => identity, Err(reply) => return reply, }; - let route = query::parse_route(path).expect("checked above"); + let Some(route) = query::parse_route(&path) else { + return Reply::error(404, "not found"); + }; query::handle(app, &identity, route, request.uri().query()).await } - (&Method::POST, path) if path.starts_with("/v1/cache/") => { - cache_request(app, request, "POST", path).await - } - (&Method::GET, "/v1/cache/summary" | "/v1/cache/entries") => { - cache_request(app, request, "GET", &path).await + Handler::CacheRead | Handler::CacheLookup | Handler::CacheWrite => { + cache_request( + app, + request, + &method, + &path, + body_limit(app, Some(route), &path), + ) + .await } - // A rewritten signed blob URL: `PUT|GET //cas/`. - // Only the in-memory store answers it. A GCS-backed server, which is - // every production deploy, falls through to 404. - (method, path) - if app.cache_store.is_memory() - && matches!(method, &Method::PUT | &Method::GET) - && path.split('/').nth(2) == Some("cas") => - { - let method = method.as_str().to_string(); - cache_request(app, request, &method, path).await + // Only the in-memory store answers a rewritten signed blob URL. A + // GCS-backed server, which is every production deploy, has none. + Handler::SignedBlob => { + if app.config.dev_blob_route + && app.cache_store.is_memory() + && path.split('/').nth(2) == Some("cas") + { + cache_request( + app, + request, + &method, + &path, + body_limit(app, Some(route), &path), + ) + .await + } else { + Reply::error(404, "not found") + } } - (method, path) if crate::ui::is_ui_path(path) => { - let assertion = request - .headers() - .get(crate::iap::IAP_HEADER) - .and_then(|value| value.to_str().ok()) - .map(str::to_string); - crate::ui::handle(app, method, path, request.uri().query(), assertion).await + Handler::Ui => { + let assertion = header_name(&request, crate::iap::IAP_HEADER); + let http_method = request.method().clone(); + crate::ui::handle(app, &http_method, &path, request.uri().query(), assertion).await } - ( - _, - "/healthz" - | "/readyz" - | "/cli-auth" - | "/v1/auth/token" - | "/v1/test-results/runs" - | "/v1/test-results/durations", - ) => Reply::error(405, "method not allowed"), - (_, path) if query::parse_route(path).is_some() => Reply::error(405, "method not allowed"), - _ => Reply::error(404, "not found"), } } -/// Which CI job is calling, from the bearer token; the error is the reply -/// to send. A person's token is valid but is for reading the cache only. -async fn authenticate(app: &App, request: &Request) -> Result { - let authorization = header(request, AUTHORIZATION); - match app.principal(authorization.as_deref()).await { - Ok(Principal::Ci(identity)) => Ok(identity), - Ok(Principal::Person(person)) => { - audit::auth_failure( - Some(&person.subject), - "person_token_not_allowed", - "a person's token may only read the cache", - ); - Err(Reply::error( - 403, - "a person's token may only read the cache", - )) +/// Who is reading, for the GET routes over test results. A CI token reads +/// its own repository. A signed-in person reads any repository on the +/// allowlist, named by `?repo=owner/name`, so the identity returned for them +/// is one for that repository and the handlers apply their usual +/// own-repository rule to it. +fn read_identity( + app: &App, + principal: Option, + request: &Request, +) -> Result { + let principal = principal.ok_or_else(|| Reply::error(401, "send a bearer token"))?; + if let Principal::Person(person) = &principal { + let pairs = query::parse_query(request.uri().query()) + .ok_or_else(|| Reply::error(400, "the query is not valid"))?; + let repo = pairs + .iter() + .find(|(key, _)| key == "repo") + .map(|(_, value)| value.as_str()); + if let Some(repo) = repo { + crate::cache::person_may_browse(app, person, repo) + .map_err(|message| Reply::error(403, message))?; } - Err(error) => Err(error.into_reply()), + return routes::identity_for(principal, repo); } + routes::identity_for(principal, None) } fn header_name(request: &Request, name: &str) -> Option { @@ -597,7 +601,13 @@ fn header(request: &Request, name: hyper::header::HeaderName) -> Optio .map(str::to_string) } -async fn cache_request(app: &App, request: Request, method: &str, path: &str) -> Reply { +async fn cache_request( + app: &App, + request: Request, + method: &str, + path: &str, + limit: usize, +) -> Reply { let authorization = header(&request, AUTHORIZATION); let query = request.uri().query().map(str::to_string); // JSON cache routes carry a small body. The rewritten blob GET does not, @@ -605,7 +615,7 @@ async fn cache_request(app: &App, request: Request, method: &str, path let body = if method == "GET" { Bytes::new() } else { - match read_body(request, app.config.max_body_bytes, app.config.body_timeout).await { + match read_body(request, limit, app.config.body_timeout).await { Ok(body) => body, Err(reply) => return reply, } diff --git a/crates/aster-srv/src/iap.rs b/crates/aster-srv/src/iap.rs index 8ec0d203..ff50569a 100644 --- a/crates/aster-srv/src/iap.rs +++ b/crates/aster-srv/src/iap.rs @@ -76,6 +76,8 @@ pub struct IapVerifier { /// Where keys come from; `None` uses only keys added with [`Self::with_key`]. jwks_url: Option, keys: RwLock>, + /// When keys were last fetched. Held for the whole fetch, so a caller + /// that arrives mid-fetch waits and then finds the fetched keys. last_fetch: Mutex>, client: reqwest::Client, } @@ -143,7 +145,7 @@ impl IapVerifier { let invalid = |reason: String| ViewerError::Unauthenticated(format!("invalid assertion: {reason}")); - let header = decode_header(token).map_err(|e| invalid(e.to_string()))?; + let header = decode_header(token).map_err(|_| invalid("it is malformed".into()))?; if header.alg != Algorithm::ES256 { return Err(invalid("assertions must be signed with ES256".into())); } @@ -153,12 +155,14 @@ impl IapVerifier { let key = self.key(&kid).await.map_err(invalid)?; let mut validation = Validation::new(Algorithm::ES256); - // The audience may be a prefix, so it is checked below. + // The audience is compared exactly below. validation.validate_aud = false; validation.set_issuer(&[IAP_ISSUER]); validation.set_required_spec_claims(&["exp", "iss", "aud"]); + validation.validate_nbf = true; + validation.leeway = 5; let claims = decode::(token, &key, &validation) - .map_err(|e| invalid(e.to_string()))? + .map_err(|_| invalid("it did not verify".into()))? .claims; if !audience_matches(audience, &claims.aud) { return Err(invalid("the assertion is for another audience".into())); @@ -189,13 +193,17 @@ impl IapVerifier { let Some(url) = &self.jwks_url else { return Err(unknown()); }; - { - let mut last = self.last_fetch.lock().await; - if last.is_some_and(|at| at.elapsed() < REFETCH_INTERVAL) { - return Err(unknown()); - } - *last = Some(Instant::now()); + // One fetch at a time, with the lock held across it. Callers that + // queued behind a fetch look again once they hold the lock, so they + // get the key it fetched instead of failing while it was in flight. + let mut last = self.last_fetch.lock().await; + if let Some(key) = self.keys.read().await.get(kid) { + return Ok(key.clone()); + } + if last.is_some_and(|at| at.elapsed() < REFETCH_INTERVAL) { + return Err(unknown()); } + *last = Some(Instant::now()); let set: JwkSet = self .client .get(url) @@ -206,28 +214,27 @@ impl IapVerifier { .json() .await .map_err(|e| format!("cannot read signing keys: {e}"))?; - let mut keys = self.keys.write().await; + // The fetched set replaces the old one: the map stays as small as the + // issuer's set, and a key the issuer withdrew stops verifying. + let mut fetched = HashMap::new(); for jwk in &set.keys { if let (Some(id), Ok(key)) = (&jwk.common.key_id, DecodingKey::from_jwk(jwk)) { - keys.insert(id.clone(), key); + fetched.insert(id.clone(), key); } } + let mut keys = self.keys.write().await; + *keys = fetched; keys.get(kid).cloned().ok_or_else(unknown) } } -/// `configured` names one backend service, matched exactly, or (ending in -/// `/`) a project, matching any of its backend services. Both sides must -/// have the full IAP audience shape, whatever configuration validation let -/// through. +/// The assertion's audience is exactly the configured backend service. A +/// prefix match would accept an assertion IAP issued for another backend +/// service of the same project. fn audience_matches(configured: &str, audience: &str) -> bool { - let (Some(want), Some(got)) = (IapAudience::parse(configured), IapAudience::parse(audience)) - else { - return false; - }; - match want.backend_service { - Some(_) => want == got, - None => want.project == got.project && got.backend_service.is_some(), + match (IapAudience::parse(configured), IapAudience::parse(audience)) { + (Some(want), Some(got)) => want.backend_service.is_some() && want == got, + _ => false, } } @@ -237,7 +244,7 @@ mod tests { use jsonwebtoken::{encode, EncodingKey, Header}; use serde_json::json; - const AUDIENCE: &str = "/projects/123456/global/backendServices/"; + const AUDIENCE: &str = "/projects/123456/global/backendServices/987"; /// A P-256 key pair made for this test process, so no private key is /// checked in: `(signing key as PKCS#8, public key as an uncompressed point)`. @@ -296,7 +303,7 @@ mod tests { } #[tokio::test] - async fn accepts_an_assertion_for_a_prefix_or_an_exact_audience() { + async fn accepts_an_assertion_for_the_configured_audience() { let prefix = verifier(&[("ASTER_SRV_IAP_AUDIENCE", AUDIENCE)]); let assertion = token(claims(), "k1"); let viewer = prefix.verify(Some(&assertion)).await.unwrap(); @@ -317,6 +324,10 @@ mod tests { let v = &v; async move { v.verify(Some(&assertion)).await } }; + // Another backend service of the same project is another audience. + let mut other_backend = claims(); + other_backend["aud"] = json!("/projects/123456/global/backendServices/988"); + assert!(check(other_backend, "k1").await.is_err()); let mut other_project = claims(); other_project["aud"] = json!("/projects/999/global/backendServices/987"); assert!(matches!( @@ -324,7 +335,7 @@ mod tests { Err(ViewerError::Unauthenticated(_)) )); let mut bare_prefix = claims(); - bare_prefix["aud"] = json!(AUDIENCE); + bare_prefix["aud"] = json!("/projects/123456/global/backendServices/"); assert!(check(bare_prefix, "k1").await.is_err()); let mut wrong_issuer = claims(); wrong_issuer["iss"] = json!("https://accounts.google.com"); @@ -414,25 +425,99 @@ mod tests { assert!(both.verify(None).await.is_err()); } + #[tokio::test] + async fn concurrent_assertions_for_an_unseen_key_share_one_slow_fetch() { + use base64::engine::general_purpose::URL_SAFE_NO_PAD as BASE64URL; + use base64::Engine; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::Arc; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + // The test key as a JWK: the uncompressed point is 0x04 || x || y. + let point = &iap_key_pair().1; + let body = json!({"keys": [{ + "kty": "EC", "crv": "P-256", "kid": "slow", "use": "sig", "alg": "ES256", + "x": BASE64URL.encode(&point[1..33]), "y": BASE64URL.encode(&point[33..65]), + }]}) + .to_string(); + let fetches = Arc::new(AtomicUsize::new(0)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}/oauth2/v1/certs", listener.local_addr().unwrap()); + tokio::spawn({ + let fetches = Arc::clone(&fetches); + async move { + loop { + let (mut stream, _) = listener.accept().await.unwrap(); + fetches.fetch_add(1, Ordering::SeqCst); + let body = body.clone(); + tokio::spawn(async move { + let mut buffer = [0u8; 2048]; + let _ = stream.read(&mut buffer).await; + // Slow enough that every caller below arrives mid-fetch. + tokio::time::sleep(Duration::from_millis(400)).await; + let _ = stream + .write_all( + format!( + "HTTP/1.1 200 OK\r\ncontent-type: application/json\r\n\ + content-length: {}\r\nconnection: close\r\n\r\n{body}", + body.len() + ) + .as_bytes(), + ) + .await; + }); + } + } + }); + + let mut v = IapVerifier::new(&config(&[("ASTER_SRV_IAP_AUDIENCE", AUDIENCE)])); + v.jwks_url = Some(url); + v.keys + .get_mut() + .insert("withdrawn".into(), DecodingKey::from_secret(b"x")); + let v = Arc::new(v); + let assertion = token(claims(), "slow"); + let mut calls = tokio::task::JoinSet::new(); + for _ in 0..8 { + let (v, assertion) = (Arc::clone(&v), assertion.clone()); + calls.spawn(async move { v.verify(Some(&assertion)).await }); + } + while let Some(result) = calls.join_next().await { + let viewer = result.unwrap(); + assert!( + viewer.is_ok(), + "a caller that arrived during the fetch must get the fetched key: {viewer:?}" + ); + } + assert_eq!( + fetches.load(Ordering::SeqCst), + 1, + "one fetch serves them all" + ); + + // A key the issuer really does not have still fails, and does not refetch within a minute. + assert!(v.verify(Some(&token(claims(), "nope"))).await.is_err()); + assert_eq!(fetches.load(Ordering::SeqCst), 1); + } + #[test] - fn audience_prefixes_need_a_trailing_slash() { - let p = "/projects/1/global/backendServices/"; - assert!(audience_matches(p, "/projects/1/global/backendServices/2")); - assert!(!audience_matches(p, p)); - assert!(!audience_matches( - p, - "/projects/12/global/backendServices/2" - )); - assert!(!audience_matches( - p, - "/projects/1/global/backendServices/2/x" - )); + fn the_audience_must_be_the_one_configured_backend_service() { let exact = "/projects/1/global/backendServices/2"; assert!(audience_matches(exact, exact)); - assert!(!audience_matches( - exact, - "/projects/1/global/backendServices/23" - )); + for other in [ + "/projects/1/global/backendServices/3", + "/projects/1/global/backendServices/23", + "/projects/12/global/backendServices/2", + "/projects/1/global/backendServices/2/x", + "/projects/1/global/backendServices/", + ] { + assert!(!audience_matches(exact, other), "{other}"); + } + // A project-wide prefix matches nothing, not even the backend + // services of that project: another backend's assertion is refused. + let prefix = "/projects/1/global/backendServices/"; + assert!(!audience_matches(prefix, exact)); + assert!(!audience_matches(prefix, prefix)); } #[test] diff --git a/crates/aster-srv/src/lib.rs b/crates/aster-srv/src/lib.rs index f82c7f9a..89e29664 100644 --- a/crates/aster-srv/src/lib.rs +++ b/crates/aster-srv/src/lib.rs @@ -21,6 +21,7 @@ pub mod log; pub mod metrics; pub mod person; pub mod raw_store; +pub mod routes; pub mod test_results; mod ui; pub mod webhooks; diff --git a/crates/aster-srv/src/person.rs b/crates/aster-srv/src/person.rs index 164e54ce..2f9d160b 100644 --- a/crates/aster-srv/src/person.rs +++ b/crates/aster-srv/src/person.rs @@ -9,7 +9,7 @@ //! | `aud` | the same URL: the token is for this server only | //! | `sub` | the stable IAP subject (`accounts.google.com:1234...`), which survives an email rename | //! | `email` | the email IAP verified at sign-in | -//! | `scope` | `cache:read` | +//! | `scope` | `read`: the cache and every dashboard dataset, never a write | //! | `iat`, `nbf`, `exp` | issue time, not-before, expiry; `exp - iat` is at most 12 hours | //! | `jti` | 128 random bits, unique per token | //! @@ -43,8 +43,12 @@ use serde::{Deserialize, Serialize}; use crate::auth::AuthError; use crate::config::{CliTokenKey, Config}; -/// The one scope a person token carries. -pub const SCOPE: &str = "cache:read"; +/// The scope a person token carries: read the cache and every test-results +/// dataset the dashboard shows. Never a write. +pub const SCOPE: &str = "read"; +/// What tokens minted before `read` existed carry. It grants the same reads, +/// so a token signed in the last 12 hours keeps working until it expires. +pub const LEGACY_SCOPE: &str = "cache:read"; /// The longest a token lives, `exp - iat`, enforced at verify time as well as /// at issue. pub const MAX_TTL_SECONDS: u64 = 12 * 60 * 60; @@ -170,8 +174,12 @@ impl TokenIssuer { .claims; let now = Utc::now().timestamp(); let leeway = LEEWAY_SECONDS as i64; - if !claims.scope.split_whitespace().any(|scope| scope == SCOPE) { - return Err(invalid("the token lacks the cache:read scope")); + if !claims + .scope + .split_whitespace() + .any(|scope| scope == SCOPE || scope == LEGACY_SCOPE) + { + return Err(invalid("the token lacks the read scope")); } if claims.sub.is_empty() || claims.email.is_empty() { return Err(invalid("the token names no person")); @@ -249,7 +257,7 @@ mod tests { "ASTER_SRV_DATABASE_URL" => Some("postgres://localhost/x".into()), "ASTER_SRV_REPOSITORIES" => Some("ArchAstro/*".into()), "ASTER_SRV_PUBLIC_URL" => Some(URL.into()), - "ASTER_SRV_IAP_AUDIENCE" => Some("/projects/1/global/backendServices/".into()), + "ASTER_SRV_IAP_AUDIENCE" => Some("/projects/1/global/backendServices/4242".into()), "ASTER_SRV_UI_EMAIL_DOMAINS" => Some("example.com".into()), "ASTER_SRV_CLI_TOKEN_KEYS" => Some(keys.clone()), _ => None, @@ -321,7 +329,7 @@ mod tests { "sub is the stable subject, not the email" ); assert_eq!(payload["email"], "ada@example.com"); - assert_eq!(payload["scope"], "cache:read"); + assert_eq!(payload["scope"], "read"); assert_eq!(payload["iat"], payload["nbf"]); assert!(payload["jti"].as_str().unwrap().len() >= 22); let header = decode_header(&issued.token).unwrap(); diff --git a/crates/aster-srv/src/routes.rs b/crates/aster-srv/src/routes.rs new file mode 100644 index 00000000..2a9ac34f --- /dev/null +++ b/crates/aster-srv/src/routes.rs @@ -0,0 +1,370 @@ +//! The route table: every path the server answers, who may call it, and +//! which handler runs. It is the single source of truth. The router in +//! the HTTP router dispatches on [`Route::handler`], and [`gate`] decides +//! from [`Route::auth`] before any handler runs or any body is read, so a +//! route cannot exist without an auth class, and a `/v1/` path that is not +//! in the table gets no answer without a valid token. +//! +//! | Class | Who | +//! |---|---| +//! | `Public` | anyone: health checks and the sign-in code exchange | +//! | `Iap` | a person behind Google IAP (the dashboard and `/cli-auth`); the handler verifies the IAP assertion | +//! | `SignedUrl` | the test-only stand-in for a signed blob URL; only the in-memory store answers it | +//! | `Ci` | a CI token (GitHub OIDC or a development static token); a person's token is refused | +//! | `CiOrPerson` | a CI token for its own repository, or a signed-in person for any served repository: the read-only routes | +//! | `CiWriter` | a CI token only; the handler also checks the writer rules (ref, workflow, event) | + +use crate::audit; +use crate::auth::{AuthError, Identity, Principal}; +use crate::App; +use crate::Reply; + +/// Who may call a route. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Auth { + Public, + Iap, + SignedUrl, + Ci, + CiOrPerson, + CiWriter, +} + +/// What answers a route. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Handler { + Healthz, + Readyz, + CliAuthConfirm, + CliAuthAuthorize, + TokenExchange, + ResultsUpload, + Durations, + ResultsRead, + CacheRead, + CacheLookup, + CacheWrite, + SignedBlob, + Ui, +} + +/// One entry of the table. In `pattern`, `:name` matches one non-empty +/// segment and a trailing `*` matches the rest of the path. +#[derive(Debug, Clone, Copy)] +pub struct Route { + pub method: &'static str, + pub pattern: &'static str, + pub auth: Auth, + pub handler: Handler, +} + +const fn route(method: &'static str, pattern: &'static str, auth: Auth, handler: Handler) -> Route { + Route { + method, + pattern, + auth, + handler, + } +} + +/// Every route. Add a new route here and nowhere else. +pub const ROUTES: &[Route] = &[ + route("GET", "/healthz", Auth::Public, Handler::Healthz), + route("GET", "/readyz", Auth::Public, Handler::Readyz), + route("GET", "/cli-auth", Auth::Iap, Handler::CliAuthConfirm), + route("POST", "/cli-auth", Auth::Iap, Handler::CliAuthAuthorize), + route( + "POST", + "/v1/auth/token", + Auth::Public, + Handler::TokenExchange, + ), + route( + "POST", + "/v1/test-results/runs", + Auth::CiWriter, + Handler::ResultsUpload, + ), + route( + "GET", + "/v1/test-results/durations", + Auth::CiOrPerson, + Handler::Durations, + ), + route( + "GET", + "/v1/test-results/tests", + Auth::CiOrPerson, + Handler::ResultsRead, + ), + route( + "GET", + "/v1/test-results/tests/:test_id", + Auth::CiOrPerson, + Handler::ResultsRead, + ), + route( + "GET", + "/v1/test-results/failures", + Auth::CiOrPerson, + Handler::ResultsRead, + ), + route( + "GET", + "/v1/test-results/quarantine", + Auth::CiOrPerson, + Handler::ResultsRead, + ), + route( + "GET", + "/v1/test-results/clusters", + Auth::CiOrPerson, + Handler::ResultsRead, + ), + route( + "GET", + "/v1/test-results/runs/:run_id/failures", + Auth::CiOrPerson, + Handler::ResultsRead, + ), + route( + "GET", + "/v1/cache/summary", + Auth::CiOrPerson, + Handler::CacheRead, + ), + route( + "GET", + "/v1/cache/entries", + Auth::CiOrPerson, + Handler::CacheRead, + ), + // A lookup is a read made with POST so it can carry many keys. + route( + "POST", + "/v1/cache/lookup", + Auth::CiOrPerson, + Handler::CacheLookup, + ), + route( + "POST", + "/v1/cache/upload", + Auth::CiWriter, + Handler::CacheWrite, + ), + route( + "POST", + "/v1/cache/commit", + Auth::CiWriter, + Handler::CacheWrite, + ), + // A rewritten signed blob URL: `PUT|GET //cas/`. + route( + "PUT", + "/:bucket/cas/:sha", + Auth::SignedUrl, + Handler::SignedBlob, + ), + route( + "GET", + "/:bucket/cas/:sha", + Auth::SignedUrl, + Handler::SignedBlob, + ), + route("*", "/ui", Auth::Iap, Handler::Ui), + route("*", "/ui/*", Auth::Iap, Handler::Ui), +]; + +fn matches(pattern: &str, path: &str) -> bool { + let mut want = pattern.split('/'); + let mut got = path.split('/'); + loop { + match (want.next(), got.next()) { + (Some("*"), Some(_)) => return true, + (Some(w), Some(g)) if w.starts_with(':') => { + if g.is_empty() { + return false; + } + } + (Some(w), Some(g)) if w == g => {} + (None, None) => return true, + _ => return false, + } + } +} + +/// The route for a request, if the table has one. +pub fn lookup(method: &str, path: &str) -> Option<&'static Route> { + ROUTES.iter().find(|route| { + (route.method == "*" || route.method == method) + && matches(route.pattern, path) + // `/v1/` is the API; a signed-URL stand-in never lives there. + && !(route.auth == Auth::SignedUrl && path.starts_with("/v1/")) + }) +} + +/// Whether some route has this path under another method. +pub fn path_known(path: &str) -> bool { + ROUTES.iter().any(|route| { + matches(route.pattern, path) && !(route.auth == Auth::SignedUrl && path.starts_with("/v1/")) + }) +} + +/// The templated route of a request, a bounded set safe as a metric and audit +/// label. It comes from the table, so a new route is labelled where it is +/// declared; the few paths answered before the table is consulted have fixed +/// labels, and anything else is `/v1/other` or `other`. +pub fn label(method: &str, path: &str) -> &'static str { + match lookup(method, path).map(|route| route.handler) { + Some(Handler::Healthz) => "/healthz", + Some(Handler::Readyz) => "/readyz", + Some(Handler::CliAuthConfirm | Handler::CliAuthAuthorize) => "/cli-auth", + Some(Handler::TokenExchange) => "/v1/auth/token", + Some(Handler::ResultsUpload) => "/v1/test-results/runs", + Some(Handler::Durations) => "/v1/test-results/durations", + Some(Handler::ResultsRead) => "/v1/test-results/query", + Some(Handler::CacheRead | Handler::CacheLookup | Handler::CacheWrite) => "/v1/cache", + Some(Handler::Ui) => "/ui", + Some(Handler::SignedBlob) => "other", + None => match path { + "/metrics" => "/metrics", + "/v1/healthz" => "/v1/healthz", + p if p.starts_with("/v1/") => "/v1/other", + _ => "other", + }, + } +} + +/// The auth gate, run before every handler of a route that needs a token. +/// +/// Deny by default: a missing, malformed, expired, mis-signed or +/// wrong-audience token is `401` whatever the route, and a `/v1/` path the +/// table does not list is `401` without a valid token and only then +/// `404`. A valid token is then held to the route's class: a person's +/// token on `Ci` and `CiWriter` routes is `403`. Handlers do their own +/// repository and writer checks on top; this is the floor. +/// +/// Every refusal is audited and counted here, whichever handler the request +/// was for, and a valid token is charged to its subject's rate limit +/// (`429`). `authorization` is the `Authorization` header value. It is never +/// put in an error. +pub async fn gate( + app: &App, + route: Option<&Route>, + authorization: Option<&str>, +) -> Result, Reply> { + let class = route.map_or(Auth::CiOrPerson, |route| route.auth); + match class { + Auth::Public | Auth::Iap | Auth::SignedUrl => Ok(None), + Auth::Ci | Auth::CiWriter | Auth::CiOrPerson => { + let principal = match app.principal(authorization).await { + Ok(principal) => principal, + Err(error) => { + let subject = match &error { + AuthError::Forbidden(repository) => Some(repository.as_str()), + _ => None, + }; + audit::auth_failure(subject, error.reason_code(), &error.to_string()); + return Err(error.into_reply()); + } + }; + let subject = match &principal { + Principal::Ci(identity) => format!("ci:{}", identity.repository), + Principal::Person(person) => format!("person:{}", person.subject), + }; + app.limits.api_subject.check(&subject).map_err(|seconds| { + audit::rate_limited("subject", Some(&subject)); + Reply::too_many(seconds) + })?; + if matches!(class, Auth::Ci | Auth::CiWriter) + && matches!(principal, Principal::Person(_)) + { + audit::auth_failure( + Some(&person_subject(&principal)), + "person_token_not_allowed", + "a person's token is read-only", + ); + return Err(Reply::error(403, "a person's token is read-only")); + } + Ok(Some(principal)) + } + } +} + +fn person_subject(principal: &Principal) -> String { + match principal { + Principal::Person(person) => person.subject.clone(), + Principal::Ci(identity) => identity.repository.clone(), + } +} + +/// Whether the router must run [`gate`] for this path: every `/v1/` path +/// except the ones the table marks as not needing a token. +pub fn needs_gate(route: Option<&Route>, path: &str) -> bool { + match route { + Some(route) => !matches!(route.auth, Auth::Public | Auth::Iap | Auth::SignedUrl), + None => path.starts_with("/v1/"), + } +} + +/// The identity a CI principal carries, or the person's, for the repository +/// they named. +pub fn identity_for(principal: Principal, repo: Option<&str>) -> Result { + match principal { + Principal::Ci(identity) => Ok(identity), + Principal::Person(_) => { + let repo = + repo.ok_or_else(|| Reply::error(400, "name the repository with ?repo=owner/name"))?; + Ok(Identity { + repository: repo.to_string(), + git_ref: None, + run_id: None, + sha: None, + workflow: None, + job_workflow: None, + event_name: None, + static_token: false, + }) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn patterns_match_whole_segments() { + assert!(matches( + "/v1/test-results/tests/:id", + "/v1/test-results/tests/a" + )); + assert!(!matches( + "/v1/test-results/tests/:id", + "/v1/test-results/tests/" + )); + assert!(!matches( + "/v1/test-results/tests/:id", + "/v1/test-results/tests/a/b" + )); + assert!(matches("/ui/*", "/ui/tests/a%2Fb")); + assert!(!matches("/ui/*", "/uix")); + assert!(matches("/:bucket/cas/:sha", "/b/cas/abc")); + assert_eq!(lookup("GET", "/ui").unwrap().handler, Handler::Ui); + assert!(lookup("DELETE", "/v1/cache/lookup").is_none()); + assert!(path_known("/v1/cache/lookup")); + } + + #[test] + fn every_v1_route_but_the_code_exchange_needs_a_token() { + for route in ROUTES.iter().filter(|r| r.pattern.starts_with("/v1/")) { + let public = route.auth == Auth::Public; + assert_eq!( + public, + route.pattern == "/v1/auth/token", + "{}", + route.pattern + ); + } + } +} diff --git a/crates/aster-srv/src/test_results/durations.rs b/crates/aster-srv/src/test_results/durations.rs index f63a7f4f..d23b76dc 100644 --- a/crates/aster-srv/src/test_results/durations.rs +++ b/crates/aster-srv/src/test_results/durations.rs @@ -223,6 +223,7 @@ mod tests { sha: None, workflow: None, job_workflow: None, + event_name: None, static_token: false, }; let (targets, day) = parse( diff --git a/crates/aster-srv/src/test_results/query.rs b/crates/aster-srv/src/test_results/query.rs index d4c00d59..68275bfc 100644 --- a/crates/aster-srv/src/test_results/query.rs +++ b/crates/aster-srv/src/test_results/query.rs @@ -1,10 +1,11 @@ //! The read API over test state: `GET /v1/test-results/tests`, //! `.../tests/:test_id`, `.../runs/:run_id/failures`, //! `.../failures?github_run=` (the failures of every Aster run in one GitHub -//! Actions run) and `.../quarantine` (the tests Aster keeps from failing -//! builds). +//! Actions run), `.../clusters` (the last week's failures grouped by failure +//! hash) and `.../quarantine` (the tests Aster keeps from failing builds). //! -//! A caller reads only its own repository's data. Lists page with an opaque +//! A CI caller reads only its own repository's data; a signed-in person +//! names any served repository with `repo` (see `http::read_identity`). Lists page with an opaque //! keyset cursor (hex of the last row's sort key), so a page costs the same //! however deep it is. @@ -15,6 +16,7 @@ use tokio_postgres::Row; use crate::auth::Identity; use crate::http::Reply; +use crate::ui::data; use crate::App; const DEFAULT_LIMIT: i64 = 100; @@ -33,6 +35,7 @@ pub enum Route { RunFailures(String), GithubFailures, Quarantine, + Clusters, } /// The route for a request path, with ids percent-decoded (a run id holds @@ -44,6 +47,7 @@ pub fn parse_route(path: &str) -> Option { ["tests"] => Some(Route::Tests), ["failures"] => Some(Route::GithubFailures), ["quarantine"] => Some(Route::Quarantine), + ["clusters"] => Some(Route::Clusters), ["tests", id] if !id.is_empty() => Some(Route::Test(percent_decode(id)?)), ["runs", id, "failures"] if !id.is_empty() => Some(Route::RunFailures(percent_decode(id)?)), _ => None, @@ -201,6 +205,7 @@ pub async fn handle(app: &App, identity: &Identity, route: Route, query: Option< Route::RunFailures(run_id) => run_failures(app, identity, &run_id, query).await, Route::GithubFailures => github_failures(app, identity, query).await, Route::Quarantine => quarantine_list(app, identity, query).await, + Route::Clusters => clusters(app, identity, query).await, }; match result { Ok(reply) | Err(Failure::Reply(reply)) => reply, @@ -343,6 +348,42 @@ async fn quarantine_list(app: &App, identity: &Identity, query: Option<&str>) -> )) } +/// `GET /v1/test-results/clusters`: the failures of the last +/// [`data::CLUSTER_DAYS`] days grouped by failure hash, most frequent first; +/// what `/ui/clusters` shows, from the same query. +async fn clusters(app: &App, identity: &Identity, query: Option<&str>) -> Handled { + let params = params(identity, query, 1, 1, false)?; + if params.state.is_some() || params.cursor.is_some() || query.unwrap_or("").contains("limit") { + return Err(Reply::error(400, "this route takes only repo").into()); + } + let since = Utc::now().date_naive() - chrono::Days::new(data::CLUSTER_DAYS); + let client = app.pool.get().await?; + let clusters = data::clusters(&client, ¶ms.repo, since).await?; + let clusters: Vec = clusters + .iter() + .map(|cluster| { + json!({ + "failure_hash": cluster.failure_hash, + "failures": cluster.failures, + "tests": cluster.tests, + "first_seen": cluster.first_seen.to_string(), + "last_seen": cluster.last_seen.to_string(), + "example_test_id": cluster.example_test_id, + "example_message": cluster.example_message, + }) + }) + .collect(); + Ok(Reply::json( + 200, + json!({ + "repo": params.repo, + "since": since.to_string(), + "days": data::CLUSTER_DAYS, + "clusters": clusters, + }), + )) +} + async fn list_tests(app: &App, identity: &Identity, query: Option<&str>) -> Handled { let params = params(identity, query, MAX_LIMIT, DEFAULT_LIMIT, false)?; let cursor = match ¶ms.cursor { diff --git a/crates/aster-srv/src/ui/mod.rs b/crates/aster-srv/src/ui/mod.rs index f25267f9..a53111f8 100644 --- a/crates/aster-srv/src/ui/mod.rs +++ b/crates/aster-srv/src/ui/mod.rs @@ -11,7 +11,7 @@ //! newest run is shown. Any repository on the allowlist can be viewed. The //! pages only read; there is no state-changing route. -mod data; +pub(crate) mod data; pub(crate) mod html; mod pages; @@ -48,11 +48,6 @@ pub enum Route { Cache, } -/// Whether `path` belongs to the dashboard. -pub fn is_ui_path(path: &str) -> bool { - path == "/ui" || path.starts_with("/ui/") -} - /// The page for a request path, with the test id percent-decoded (it may /// hold `/`, so it arrives as `%2F`). pub fn parse_route(path: &str) -> Option { @@ -318,10 +313,4 @@ mod tests { assert_eq!(parse_route(bad), None, "{bad}"); } } - - #[test] - fn recognizes_dashboard_paths() { - assert!(is_ui_path("/ui") && is_ui_path("/ui/") && is_ui_path("/ui/x")); - assert!(!is_ui_path("/uix") && !is_ui_path("/v1/ui")); - } } diff --git a/crates/aster-srv/tests/cache.rs b/crates/aster-srv/tests/cache.rs index 59af49dd..1526d5cd 100644 --- a/crates/aster-srv/tests/cache.rs +++ b/crates/aster-srv/tests/cache.rs @@ -61,7 +61,7 @@ async fn server() -> Option<(common::Server, Arc)> { ("ASTER_SRV_CACHE_DAYS", "14"), ( "ASTER_SRV_IAP_AUDIENCE", - "/projects/123456/global/backendServices/", + "/projects/123456/global/backendServices/4242", ), ("ASTER_SRV_UI_EMAIL_DOMAINS", "example.com"), ( @@ -823,7 +823,7 @@ async fn browsing_routes_authorize_viewers_and_ci() { None, ) .await; - assert_eq!(status, 404); + assert_eq!(status, 405); server.stop().await; } diff --git a/crates/aster-srv/tests/cli_auth.rs b/crates/aster-srv/tests/cli_auth.rs index 73833866..7f89b105 100644 --- a/crates/aster-srv/tests/cli_auth.rs +++ b/crates/aster-srv/tests/cli_auth.rs @@ -30,7 +30,7 @@ fn server_env() -> Vec<(&'static str, &'static str)> { ), ( "ASTER_SRV_IAP_AUDIENCE", - "/projects/123456/global/backendServices/", + "/projects/123456/global/backendServices/4242", ), ("ASTER_SRV_UI_EMAIL_DOMAINS", "example.com"), // These tests exchange many codes from one address. @@ -671,26 +671,193 @@ async fn a_person_reads_the_cache_and_nothing_else() { ) .await; assert_eq!(status, 403); - // Test results are CI's, not people's. - for (method, path) in [ - ("POST", "/v1/test-results/runs"), - ("GET", "/v1/test-results/durations?target=%2F%2Fa%3Ab"), - ("GET", "/v1/test-results/tests"), - ("GET", "/v1/test-results/quarantine"), + // A person without a repository named has nothing to read. + assert_eq!(get("/v1/test-results/tests").await, 400); + // Uploading results is CI's. + let response = browser() + .post(format!("{}/v1/test-results/runs", server.url)) + .header("authorization", &person) + .body("{}") + .send() + .await + .unwrap(); + assert_eq!(response.status().as_u16(), 403); + server.stop().await; +} + +/// What `/ui` shows, as JSON: dataset, path, and the proof it is real. +#[tokio::test] +async fn a_person_reads_every_dashboard_dataset_and_writes_nothing() { + use chrono::{Duration, Utc}; + use common::{seed, Run}; + + let Some(server) = start().await else { + return; + }; + let now = Utc::now(); + let hours = |h: i64| now - Duration::hours(h); + seed( + &server, + &Run::new("r1", "main", hours(30)) + .result("flaky one", "pass", None) + .result("broken one", "fail", Some("hash-b")), + ) + .await; + seed( + &server, + &Run::new("r2", "main", hours(20)) + .attempt("flaky one", "fail", 1, Some("hash-a")) + .attempt("flaky one", "pass", 2, None) + .result("broken one", "fail", Some("hash-b")), + ) + .await; + // Another repository's data, which a person names only by asking for it. + let mut other = Run::new("o1", "main", hours(1)).result("z", "pass", None); + other.repo = "ArchAstro/other"; + seed(&server, &other).await; + server + .client() + .await + .batch_execute(&format!( + "UPDATE test_results.tests SET state = 'flaky', flake_score = 5 \ + WHERE repo = '{REPO}' AND test_id = 'flaky one'; \ + UPDATE test_results.tests SET state = 'broken', broken_since_sha = 'r1' \ + WHERE repo = '{REPO}' AND test_id = 'broken one'; \ + INSERT INTO test_results.quarantine (repo, test_id, entered_at, expires_at, runs) \ + VALUES ('{REPO}', 'flaky one', now(), now() + interval '30 days', 2)" + )) + .await + .unwrap(); + + let person = person_bearer(&server, ADA); + let q = "repo=ArchAstro%2Ffirstlanding"; + let ci = format!("Bearer {}", common::token(REPO)); + let day = Utc::now().date_naive(); + // (dataset, path, a check on what the person reads) + type Check = fn(&Value) -> bool; + let datasets: Vec<(&str, String, Check)> = vec![ + ( + "flaky tests", + format!("/v1/test-results/tests?state=flaky&{q}"), + |b| b["tests"][0]["test_id"] == "flaky one", + ), + ( + "broken tests", + format!("/v1/test-results/tests?state=broken&{q}"), + |b| b["tests"][0]["test_id"] == "broken one", + ), + ( + "failure clusters", + format!("/v1/test-results/clusters?{q}"), + |b| b["clusters"][0]["failure_hash"] == "hash-b" && b["clusters"][0]["failures"] == 2, + ), + ( + "quarantine", + format!("/v1/test-results/quarantine?{q}"), + |b| b["tests"][0]["test_id"] == "flaky one", + ), + ( + "one test's history", + format!("/v1/test-results/tests/flaky%20one?{q}"), + |b| b["test"]["test_id"] == "flaky one", + ), + ( + "a run's failures", + format!("/v1/test-results/runs/r2/failures?{q}"), + |b| b["failures"].is_array(), + ), + ( + "durations", + format!("/v1/test-results/durations?target=%2F%2Fa%3Atest&day={day}&{q}"), + |b| b["files"].is_array(), + ), + ("cache summary", format!("/v1/cache/summary?{q}"), |b| { + b["repo"] == REPO + }), + ( + "cache entries", + format!("/v1/cache/entries?{q}&target=%2F%2Fa%3Ab"), + |b| b["entries"].is_array(), + ), + ]; + for (dataset, path, check) in &datasets { + let (status, body) = server.get_json(path, Some(&person), None).await; + assert_eq!(status, 200, "{dataset}: {body}"); + assert!(check(&body), "{dataset}: {body}"); + } + // A CI token reads its own repository's data, with or without `repo`. + let (status, body) = server + .get_json("/v1/test-results/clusters", Some(&ci), None) + .await; + assert_eq!(status, 200, "{body}"); + assert_eq!(body["clusters"][0]["failure_hash"], "hash-b"); + let (status, _) = server + .get_json( + "/v1/test-results/tests?repo=ArchAstro/other", + Some(&ci), + None, + ) + .await; + assert_eq!( + status, 403, + "a CI token still reads only its own repository" + ); + + // The person's repository is the one named, and only a served one. + let (_, theirs) = server + .get_json( + "/v1/test-results/tests?repo=ArchAstro/other", + Some(&person), + None, + ) + .await; + assert_eq!(theirs["tests"][0]["test_id"], "z"); + for path in [ + "/v1/test-results/tests?repo=evil/x", + "/v1/test-results/clusters?repo=evil/x", + "/v1/test-results/quarantine?repo=evil/x", ] { - let client = browser(); - let url = format!("{}{path}", server.url); - let request = if method == "POST" { - client.post(url).body("{}") - } else { - client.get(url) - }; - let response = request - .header("authorization", &person) - .send() - .await - .unwrap(); - assert_eq!(response.status().as_u16(), 403, "{method} {path}"); + assert_eq!(server.get_json(path, Some(&person), None).await.0, 403); + } + // Another domain's person, whose token this server issued earlier. + let outsider = person_bearer(&server, "eve@other.test"); + for (dataset, path, _) in &datasets { + let (status, _) = server.get_json(path, Some(&outsider), None).await; + assert_eq!(status, 403, "{dataset}"); + } + // No token at all. + for (dataset, path, _) in &datasets { + assert_eq!(server.get_json(path, None, None).await.0, 401, "{dataset}"); + } + // Writes are refused whatever the person names. + let digest = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + let manifest = BASE64STD.encode( + serde_json::to_vec(&json!({"schema": 1, "key": "k", "target": "//a:b", "outputs": []})) + .unwrap(), + ); + for (path, body) in [ + ("/v1/test-results/runs", json!({})), + ( + "/v1/cache/upload", + json!({"repo": REPO, "digests": [digest]}), + ), + ( + "/v1/cache/commit", + json!({"repo": REPO, "key": "k", "manifest": manifest}), + ), + ] { + let (status, response) = server.post_json(path, &body, Some(&person), None).await; + assert_eq!(status, 403, "POST {path}: {response}"); + } + for path in [ + format!("/v1/test-results/tests?{q}"), + format!("/v1/test-results/clusters?{q}"), + format!("/v1/test-results/quarantine?{q}"), + ] { + let (status, _) = server + .post_json(&path, &json!({}), Some(&person), None) + .await; + assert_eq!(status, 405, "POST {path}"); } server.stop().await; } @@ -740,9 +907,18 @@ async fn a_person_token_that_is_expired_forged_misdirected_or_rotated_out_is_ref assert_eq!( lookup(sign(claims(&|_| {}), "t1", secret)).await, 200, - "control" + "control: the scope earlier releases minted" + ); + assert_eq!( + lookup(sign(claims(&|c| c["scope"] = json!("read")), "t1", secret)).await, + 200, + "the scope this release mints" ); for (what, bearer) in [ + ( + "another scope", + sign(claims(&|c| c["scope"] = json!("write")), "t1", secret), + ), ( "expired", sign( diff --git a/crates/aster-srv/tests/common/mod.rs b/crates/aster-srv/tests/common/mod.rs index ebfe144b..6bcbcd56 100644 --- a/crates/aster-srv/tests/common/mod.rs +++ b/crates/aster-srv/tests/common/mod.rs @@ -18,7 +18,7 @@ use serde_json::{json, Value}; const PRIVATE: &[u8] = include_bytes!("../fixtures/test-oidc-key.pem"); const PUBLIC: &[u8] = include_bytes!("../fixtures/test-oidc-key.pub.pem"); /// The audience dashboard tests configure and sign for. -pub const IAP_AUDIENCE: &str = "/projects/123456/global/backendServices/"; +pub const IAP_AUDIENCE: &str = "/projects/123456/global/backendServices/4242"; pub const REPO: &str = "ArchAstro/firstlanding"; /// What the test server answers to, and how it signs person tokens. pub const PUBLIC_URL: &str = "https://aster-srv.test"; @@ -181,6 +181,7 @@ async fn launch( { Some(CLI_TOKEN_KEYS.into()) } + "ASTER_SRV_DEV_BLOB_ROUTE" => Some("1".into()), _ => None, }), }) @@ -350,7 +351,7 @@ pub fn iap_subject(email: &str) -> String { /// An IAP assertion for `email`, signed with the fixture key. pub fn iap_assertion(email: &str) -> String { - iap_assertion_for(email, &format!("{IAP_AUDIENCE}4242")) + iap_assertion_for(email, IAP_AUDIENCE) } /// [`iap_assertion`] addressed to `audience`. @@ -360,12 +361,12 @@ pub fn iap_assertion_for(email: &str, audience: &str) -> String { /// An assertion with no `sub` claim at all. pub fn iap_assertion_without_subject(email: &str) -> String { - iap_assertion_full(email, None, &format!("{IAP_AUDIENCE}4242")) + iap_assertion_full(email, None, IAP_AUDIENCE) } /// An assertion for a chosen subject, as when an account's email changes. pub fn iap_assertion_with_subject(email: &str, subject: &str) -> String { - iap_assertion_full(email, Some(subject), &format!("{IAP_AUDIENCE}4242")) + iap_assertion_full(email, Some(subject), IAP_AUDIENCE) } fn iap_assertion_full(email: &str, subject: Option<&str>, audience: &str) -> String { @@ -394,6 +395,7 @@ pub fn token_with_ref(repository: &str, workflow_ref: Option<&str>) -> String { "aud": "aster-srv", "exp": Utc::now().timestamp() + 300, "repository": repository, + "event_name": "push", }); if let Some(workflow_ref) = workflow_ref { claims["ref"] = json!(workflow_ref); @@ -416,6 +418,7 @@ pub fn token_with_claims(repository: &str, extra: Value) -> String { "aud": "aster-srv", "exp": Utc::now().timestamp() + 300, "repository": repository, + "event_name": "push", }); for (name, value) in extra.as_object().expect("claims are an object") { claims[name] = value.clone(); diff --git a/crates/aster-srv/tests/fixtures/test-other-key.pem b/crates/aster-srv/tests/fixtures/test-other-key.pem new file mode 100644 index 00000000..c0d53b59 --- /dev/null +++ b/crates/aster-srv/tests/fixtures/test-other-key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAuFfI1aIpPduEXnBM+6Qca4rVqHXlifNWQJNfNyJyDPrapEb6 +OF5JJ2o3wB3kfe5W2yfA6NVl6SyrvHktsbzdiUnMubKuvDv9Iz3YLgL82eWJeu7Z +0xm6d06nsJaHZGbgq/oYXkCeAZrdsfk8j+wLjolvfJGNHJmbxBCRXvClpcXw08CL +6QxohHhcepeWnJOpVRmTK6reNyxH4VeBTkS0aPMSw6R9bW2rkZuoCJCiuoTre8hv +Z2HgkisqbDSV8HjGj+JU3k9xOxSYmZ1AapfKS3agGnbiMILWr0/wNzKQluo5lI3C +JZmOtdPXBtsBRPbyrZ2SuFsYQzpUYMuWJ81GdQIDAQABAoIBACJfW/dZsU7za48g +aXpSc2oqDQTIVvCOCvsnf42MQq/g5nvTMDzqd8F9sL+F02EX2+IpW3MOt13vEmam +QbrueHBs7luB0csPdHfpxDFwJS2L+0KM3tjZ50bvx5zJtJ0oAaSPkZOhgNTTx5qz +2nMLlGwTfFJB4Yn/qme4nyKQxJJQMS6+PJi71UHJzWzGSXVm3Q6Afg+L3VBdjs1R +mxAby9ugNhancZKQ9zDl+afQwCXUyekuuPCO4j+dcL5pGymaRd0WpgjvxSNmrs++ +g2XHrKgfi+LglHwXyVKVIwWhYlxoK6gi0dJ7f9/i9Zs6xuwiVbRHfSjpQro/kopi +9HYRmgECgYEA4ldk2ujBimKCgmeVqSZhHoQNCxk1rERttg9Fe58O3R/8XMIE62pX +Z391r4AUYTYDxakOTnqeFCbzJYIOIEXq8mES0bueZwHfZexf7+ZuWmtxtTeIWOIb +1diPKc4K00zXS3GLfxRoTJS5RS2iofCqHEq2OYpnFXOTIPc9NinXHOECgYEA0H+N ++8UJ95Kv5dXs7nlKil53RvYCoGNOt83fO7KWCmnPirUALwFxEKoMRoK8nOLXIQFj +hZSHvbO1tQ8mV8cZRz/k+4+BkTzmDBRIOeKJatn7R9m0T45VdcUap9nFHEcWiDut +VaSQUY+Snp+kwrxEFYO6Js8gxCQn+vnSxwp86BUCgYB0fAWro533WjtQYX7kTpws +2EjeLVsyDQVpbPKVbIZ5xrxaAIAHp/LTkQVADOpwHRxT3W0/dltkg6ce5o/ejB0e +IFdPGP+sg4o1KjE1haHFZwZd/PZux+8LXyhOm3t2w1UJvqXMAPSsdN3kfFf2f6UM +ambdthzXLWPK03LtLRtIAQKBgG+K3fXEqxlkw2+tK+1EvGkcMqmm8S7HEhTpMbUp +THXTt7GYNbnwBoIIxzpttOQlwzuoF4seNCR74Qk1SqkJHe/tMd6XHpUJxHTP4Pf+ +xs+AQJtmz70EA5pW0XTc30PKkDZf3tKDkKXhrmnoNGft+mqcJ1md1AS65xywzF+p +r7slAoGBAJ8EfGVMqx0ez7B9tNOXVJQRFn6e/UdTC/CW6rqiLIXnSls6MtxsRdPd +Q/g+DcPqi+J+PgVGLiZkHTW4Dy/yoghN2oIUMtH3VVtQgqv2zbUnXElpYoQw062X +lUL+NZA9feqdBAcJvZcdspGT6Bev4Tc1SleZDFEwjxI9HOVfHP0P +-----END RSA PRIVATE KEY----- diff --git a/crates/aster-srv/tests/hardening.rs b/crates/aster-srv/tests/hardening.rs index 51d709c8..33292556 100644 --- a/crates/aster-srv/tests/hardening.rs +++ b/crates/aster-srv/tests/hardening.rs @@ -43,7 +43,7 @@ async fn start_with(extra: &[(&str, &str)]) -> Option { ), ( "ASTER_SRV_IAP_AUDIENCE", - "/projects/123456/global/backendServices/", + "/projects/123456/global/backendServices/4242", ), ("ASTER_SRV_UI_EMAIL_DOMAINS", "example.com"), ]); @@ -208,7 +208,10 @@ async fn api_listener_serves_only_v1_while_the_main_port_is_unchanged() { "/metrics through IAP is hidden" ); assert_eq!(get(format!("{main}/ui")).await.status(), 200); - assert_eq!(get(format!("{main}/v1/healthz")).await.status(), 404); + // The API listener's bare health check is not a route here. Any `/v1/` + // path outside the route table is behind the auth gate, so without a + // token the main port answers 401, not the API port's 200. + assert_eq!(get(format!("{main}/v1/healthz")).await.status(), 401); // Phase 2: a person signs in on the main port, the CLI redeems and uses // the token on the API port. @@ -268,12 +271,14 @@ async fn api_listener_serves_only_v1_while_the_main_port_is_unchanged() { assert_eq!(hsts.unwrap(), HSTS); } } - // Paths that begin with /v1/ but name no API route are 404 as well. - for path in [ - "/v1/../ui", - "/v1/../cli-auth", - "/v1/nothing", - "/v1/auth/other", + // Paths that begin with /v1/ but name no API route are behind the auth + // gate: 401 without a token (the IAP header is not a token), never a + // page. A client normalizes `/v1/../x` to `/x`, which is not /v1 at all. + for (path, status) in [ + ("/v1/../ui", 404), + ("/v1/../cli-auth", 404), + ("/v1/nothing", 401), + ("/v1/auth/other", 401), ] { let response = http .get(format!("{api}{path}")) @@ -281,7 +286,7 @@ async fn api_listener_serves_only_v1_while_the_main_port_is_unchanged() { .send() .await .unwrap(); - assert_eq!(response.status(), 404, "{path}"); + assert_eq!(response.status(), status, "{path}"); } assert_eq!( http.post(format!("{api}/v1/healthz")) @@ -289,7 +294,7 @@ async fn api_listener_serves_only_v1_while_the_main_port_is_unchanged() { .await .unwrap() .status(), - 404 + 401 ); // The API port refuses a bad exchange like the main port does. assert_eq!(bad_exchange(&api, None).await.status(), 400); diff --git a/crates/aster-srv/tests/query.rs b/crates/aster-srv/tests/query.rs index da8c7ce9..633cb11f 100644 --- a/crates/aster-srv/tests/query.rs +++ b/crates/aster-srv/tests/query.rs @@ -141,14 +141,19 @@ async fn lists_tests_by_score_with_a_cursor_and_filters() { .await; assert_eq!(status, 400, "{bad}: {body}"); } - let response = aster_srv::http_client() - .build() - .unwrap() - .post(format!("{}/v1/test-results/tests", server.url)) - .send() - .await - .unwrap(); - assert_eq!(response.status().as_u16(), 405); + let post = |token: Option| { + let url = format!("{}/v1/test-results/tests", server.url); + async move { + let mut request = aster_srv::http_client().build().unwrap().post(url); + if let Some(token) = token { + request = request.bearer_auth(token); + } + request.send().await.unwrap().status().as_u16() + } + }; + // Auth comes first: no token is 401 even for a method the route lacks. + assert_eq!(post(None).await, 401); + assert_eq!(post(Some(common::token(REPO))).await, 405); server.stop().await; } diff --git a/crates/aster-srv/tests/routes.rs b/crates/aster-srv/tests/routes.rs new file mode 100644 index 00000000..8a7e0c0f --- /dev/null +++ b/crates/aster-srv/tests/routes.rs @@ -0,0 +1,699 @@ +//! The route table's guarantees, against a real server and Postgres. +//! +//! Every test walks `aster_srv::routes::ROUTES`, so a route added to the +//! table is covered without editing a test, and a route that skips the auth +//! gate fails them. + +mod common; + +use std::sync::Arc; + +use aster_srv::cache::{BlobStore, FixedSigner, Signer}; +use aster_srv::routes::{Auth, Route, ROUTES}; +use base64::engine::general_purpose::URL_SAFE_NO_PAD as BASE64URL; +use base64::Engine; +use common::{iap_assertion, person_bearer, token_with_claims, Server, REPO}; +use jsonwebtoken::{encode, Algorithm, EncodingKey, Header}; +use serde_json::{json, Value}; + +const CI_WORKFLOW: &str = "ArchAstro/firstlanding/.github/workflows/ci.yml@refs/heads/main"; +const OTHER_KEY: &[u8] = include_bytes!("fixtures/test-other-key.pem"); +const PUBLIC_KEY: &[u8] = include_bytes!("fixtures/test-oidc-key.pub.pem"); +const PRIVATE_KEY: &[u8] = include_bytes!("fixtures/test-oidc-key.pem"); +const DIGEST: &str = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + +async fn start() -> Option { + let signer = Arc::new(FixedSigner { + email: "cache-test@example.iam.gserviceaccount.com".into(), + signature: b"test-signature".to_vec(), + signed: Default::default(), + }); + let store = BlobStore::memory("aster-cache-test", signer as Arc); + common::start_with( + &[ + ("ASTER_SRV_CACHE_BUCKET", "aster-cache-test"), + ( + "ASTER_SRV_CACHE_SIGNER", + "cache-test@example.iam.gserviceaccount.com", + ), + ( + "ASTER_SRV_IAP_AUDIENCE", + "/projects/123456/global/backendServices/4242", + ), + ("ASTER_SRV_UI_EMAIL_DOMAINS", "example.com"), + ( + "ASTER_SRV_CACHE_WRITER_WORKFLOWS", + "ArchAstro/firstlanding/.github/workflows/ci.yml@", + ), + ("ASTER_SRV_REPOSITORY_OWNER_IDS", "ArchAstro/*=4242"), + ], + Some(store), + ) + .await +} + +/// A route's pattern with each `:name` filled in. +fn path_of(route: &Route) -> String { + route + .pattern + .split('/') + .map(|segment| { + if segment.starts_with(':') { + "x" + } else { + segment + } + }) + .collect::>() + .join("/") +} + +/// Every route under `/v1/` that is not the code exchange, plus a path the +/// table does not list, which the gate must also refuse. +fn v1_routes() -> Vec<(String, String, Auth)> { + let mut routes: Vec<(String, String, Auth)> = ROUTES + .iter() + .filter(|route| route.pattern.starts_with("/v1/") && route.auth != Auth::Public) + .map(|route| (route.method.to_string(), path_of(route), route.auth)) + .collect(); + routes.push(("GET".into(), "/v1/not-a-route".into(), Auth::CiOrPerson)); + routes.push(("DELETE".into(), "/v1/cache/lookup".into(), Auth::CiOrPerson)); + routes +} + +async fn send( + server: &Server, + method: &str, + path: &str, + headers: &[(&str, &str)], +) -> (u16, String) { + let client = aster_srv::http_client().build().unwrap(); + let url = format!("{}{path}", server.url); + let mut request = match method { + "POST" => client.post(url).body("{}"), + "PUT" => client.put(url).body("{}"), + "DELETE" => client.delete(url), + _ => client.get(url), + }; + for (name, value) in headers { + request = request.header(*name, *value); + } + let response = request.send().await.unwrap(); + (response.status().as_u16(), response.text().await.unwrap()) +} + +fn jwt(header: Header, key: &EncodingKey, claims: Value) -> String { + encode(&header, &claims, key).unwrap() +} + +fn oidc_claims() -> Value { + json!({ + "iss": "https://token.actions.githubusercontent.com", "aud": "aster-srv", + "exp": chrono::Utc::now().timestamp() + 300, "repository": REPO, + "ref": "refs/heads/main", "event_name": "push", + "job_workflow_ref": CI_WORKFLOW, "workflow_ref": CI_WORKFLOW, + }) +} + +fn rs256(key: &[u8], claims: Value) -> String { + let mut header = Header::new(Algorithm::RS256); + header.kid = Some("test".into()); + jwt(header, &EncodingKey::from_rsa_pem(key).unwrap(), claims) +} + +/// Tokens that must never open a route: `(what, header value)`. +fn bad_credentials() -> Vec<(&'static str, Option)> { + let with = |change: &dyn Fn(&mut Value)| { + let mut claims = oidc_claims(); + change(&mut claims); + claims + }; + let bearer = |token: String| Some(format!("Bearer {token}")); + let hs256 = |kid: &str, secret: &[u8], claims: Value| { + let mut header = Header::new(Algorithm::HS256); + header.kid = Some(kid.into()); + jwt(header, &EncodingKey::from_secret(secret), claims) + }; + let person_claims = |change: &dyn Fn(&mut Value)| { + let mut claims = json!({ + "iss": common::PUBLIC_URL, "aud": "aster-cli", "sub": "ada@example.com", + "scope": "read", "iat": chrono::Utc::now().timestamp(), + "exp": chrono::Utc::now().timestamp() + 300, "jti": "j", + }); + change(&mut claims); + claims + }; + let none = format!( + "{}.{}.", + BASE64URL.encode(r#"{"alg":"none","typ":"JWT","kid":"test"}"#), + BASE64URL.encode(oidc_claims().to_string()), + ); + vec![ + ("no credentials", None), + ("a garbage token", bearer("garbage-token-0123456789".into())), + ("an empty bearer", Some("Bearer ".into())), + ("another scheme", Some("Basic YTpi".into())), + ( + "an expired token", + bearer(rs256( + PRIVATE_KEY, + with(&|c| c["exp"] = json!(chrono::Utc::now().timestamp() - 3600)), + )), + ), + ( + "a token signed with the wrong key", + bearer(rs256(OTHER_KEY, oidc_claims())), + ), + ( + "a token that is not valid yet (nbf)", + bearer(rs256( + PRIVATE_KEY, + with(&|c| c["nbf"] = json!(chrono::Utc::now().timestamp() + 3600)), + )), + ), + ("alg=none", bearer(none)), + ( + "an HS256 token keyed with the RSA public key (algorithm confusion)", + bearer(hs256("test", PUBLIC_KEY, oidc_claims())), + ), + ( + "an HS256 token keyed with the RSA private key", + bearer(hs256("test", PRIVATE_KEY, oidc_claims())), + ), + ( + "an HS256 token for the person key id signed with the RSA public key", + bearer(hs256("t1", PUBLIC_KEY, person_claims(&|_| {}))), + ), + ( + "an OIDC token for another audience", + bearer(rs256( + PRIVATE_KEY, + with(&|c| c["aud"] = json!("someone-else")), + )), + ), + ( + "an OIDC token from another issuer", + bearer(rs256( + PRIVATE_KEY, + with(&|c| c["iss"] = json!("https://evil.example")), + )), + ), + ( + "a person token for the wrong audience", + bearer(hs256( + "t1", + common::CLI_TOKEN_KEYS.split_once(':').unwrap().1.as_bytes(), + person_claims(&|c| c["aud"] = json!("aster-srv")), + )), + ), + ( + "a person token from the wrong issuer", + bearer(hs256( + "t1", + common::CLI_TOKEN_KEYS.split_once(':').unwrap().1.as_bytes(), + person_claims(&|c| c["iss"] = json!("https://evil.example")), + )), + ), + ] +} + +#[tokio::test] +async fn every_v1_route_answers_401_to_every_kind_of_bad_credential() { + let Some(server) = start().await else { + return; + }; + // IAP in front of the server is not a credential on /v1: a perfectly + // signed assertion, or headers claiming IAP's identity, open nothing. + let forged: Vec<(&str, Vec<(&str, String)>)> = vec![ + ( + "a valid IAP assertion and no token", + vec![("x-goog-iap-jwt-assertion", iap_assertion("ada@example.com"))], + ), + ( + "forged x-goog-iap headers", + vec![ + ( + "x-goog-authenticated-user-email", + "accounts.google.com:ada@example.com".into(), + ), + ("x-goog-iap-jwt-assertion", "forged.iap.assertion".into()), + ( + "x-goog-authenticated-user-id", + "accounts.google.com:1".into(), + ), + ], + ), + ]; + for (method, path, _) in v1_routes() { + let path = format!("{path}?repo=ArchAstro%2Ffirstlanding"); + for (what, authorization) in bad_credentials() { + let headers: Vec<(&str, &str)> = authorization + .as_deref() + .map(|value| vec![("authorization", value)]) + .unwrap_or_default(); + let (status, body) = send(&server, &method, &path, &headers).await; + assert_eq!(status, 401, "{method} {path} with {what}: {body}"); + // Whatever is wrong with it, the reply says only that. The + // reason is logged, not sent. + if authorization.is_some() { + assert!( + body == r#"{"error":"the token is not valid"}"# + || body == r#"{"error":"send a bearer token"}"#, + "{method} {path} with {what}: {body}" + ); + } + // The token is never echoed back. + if let Some(value) = &authorization { + let token = value.rsplit(' ').next().unwrap_or_default(); + assert!( + token.is_empty() || !body.contains(token), + "{method} {path} with {what} echoed the token: {body}" + ); + } + } + for (what, headers) in &forged { + let headers: Vec<(&str, &str)> = + headers.iter().map(|(n, v)| (*n, v.as_str())).collect(); + let (status, body) = send(&server, &method, &path, &headers).await; + assert_eq!(status, 401, "{method} {path} with {what}: {body}"); + } + } + server.stop().await; +} + +#[tokio::test] +async fn the_code_exchange_is_the_only_v1_route_without_a_token_and_never_says_200_without_a_code() +{ + let Some(server) = start().await else { + return; + }; + let public: Vec<&str> = ROUTES + .iter() + .filter(|route| route.pattern.starts_with("/v1/") && route.auth == Auth::Public) + .map(|route| route.pattern) + .collect(); + assert_eq!(public, ["/v1/auth/token"]); + let client = aster_srv::http_client().build().unwrap(); + for body in [ + "", + "grant_type=authorization_code&code=nope&code_verifier=nope", + "grant_type=authorization_code&code=&code_verifier=", + "code=nope", + ] { + let response = client + .post(format!("{}/v1/auth/token", server.url)) + .header("content-type", "application/x-www-form-urlencoded") + .body(body) + .send() + .await + .unwrap(); + let status = response.status().as_u16(); + assert_ne!(status, 200, "{body:?}"); + assert!(status == 400, "{body:?}: {status}"); + let reply: Value = response.json().await.unwrap(); + assert_eq!(reply["error"], "invalid_grant", "{body:?}"); + } + server.stop().await; +} + +/// The reads a person's token may make: the whole allowlist, spelled out so +/// a new route classed `CiOrPerson` is a decision made here too. +const PERSON_READS: [(&str, &str); 10] = [ + ("GET", "/v1/test-results/durations"), + ("GET", "/v1/test-results/tests"), + ("GET", "/v1/test-results/tests/:test_id"), + ("GET", "/v1/test-results/failures"), + ("GET", "/v1/test-results/quarantine"), + ("GET", "/v1/test-results/clusters"), + ("GET", "/v1/test-results/runs/:run_id/failures"), + ("GET", "/v1/cache/summary"), + ("GET", "/v1/cache/entries"), + ("POST", "/v1/cache/lookup"), +]; + +#[tokio::test] +async fn a_person_reaches_only_the_read_allowlist_and_every_other_route_is_403() { + let Some(server) = start().await else { + return; + }; + let allowed: Vec<(&str, &str)> = ROUTES + .iter() + .filter(|route| route.auth == Auth::CiOrPerson) + .map(|route| (route.method, route.pattern)) + .collect(); + let expected: Vec<(&str, &str)> = PERSON_READS.to_vec(); + assert_eq!(allowed, expected, "the person allowlist changed"); + + let person = person_bearer(&server, "ada@example.com"); + let mut checked = 0; + for route in ROUTES.iter().filter(|r| r.pattern.starts_with("/v1/")) { + if route.auth == Auth::Public { + continue; + } + let path = path_of(route); + let post_body = |repo: &str| json!({"repo": repo, "keys": ["k"]}).to_string(); + for repo in [REPO, "evil/x"] { + let url = format!("{}{path}?repo={}", server.url, repo.replace('/', "%2F")); + let client = aster_srv::http_client().build().unwrap(); + let request = match route.method { + "POST" => client + .post(&url) + .header("content-type", "application/json") + .body(post_body(repo)), + _ => client.get(&url), + }; + let status = request + .header("authorization", &person) + .send() + .await + .unwrap() + .status() + .as_u16(); + let what = format!("{} {} for {repo}", route.method, route.pattern); + match (route.auth, repo) { + (Auth::CiOrPerson, REPO) => { + assert!(![401, 403].contains(&status), "{what}: {status}"); + } + _ => assert_eq!(status, 403, "{what}"), + } + } + checked += 1; + } + assert!(checked >= 13, "the walk covered {checked} routes"); + server.stop().await; +} + +async fn write_status(server: &Server, token: &str, path: &str) -> u16 { + let manifest = base64::engine::general_purpose::STANDARD + .encode(json!({"schema": 1, "key": "k", "target": "//a:b", "outputs": []}).to_string()); + let body = match path { + "/v1/cache/upload" => json!({"repo": REPO, "digests": [DIGEST]}).to_string(), + "/v1/cache/commit" => json!({"repo": REPO, "key": "k", "manifest": manifest}).to_string(), + _ => String::new(), + }; + aster_srv::http_client() + .build() + .unwrap() + .post(format!("{}{path}", server.url)) + .header("authorization", format!("Bearer {token}")) + .header("content-type", "application/json") + .body(body) + .send() + .await + .unwrap() + .status() + .as_u16() +} + +#[tokio::test] +async fn cache_writes_need_a_push_to_main_from_the_ci_workflow_in_an_allowed_repository() { + let Some(server) = start().await else { + return; + }; + let base = json!({ + "ref": "refs/heads/main", "event_name": "push", + "job_workflow_ref": CI_WORKFLOW, "workflow_ref": CI_WORKFLOW, + "repository_owner_id": "4242", + }); + let variant = |change: &Value| { + let mut claims = base.clone(); + for (name, value) in change.as_object().unwrap() { + claims[name] = value.clone(); + } + token_with_claims(REPO, claims) + }; + assert_eq!( + write_status(&server, &variant(&json!({})), "/v1/cache/upload").await, + 200, + "control: a push to main from ci.yml writes" + ); + let evil = "ArchAstro/firstlanding/.github/workflows/evil.yml@refs/heads/main"; + let cases: Vec<(&str, Value)> = vec![ + ( + "a pull request", + json!({"ref": "refs/pull/7/merge", "event_name": "pull_request"}), + ), + ( + "a pull request that claims main", + json!({"event_name": "pull_request"}), + ), + ( + "pull_request_target", + json!({"event_name": "pull_request_target"}), + ), + ("a feature branch", json!({"ref": "refs/heads/feature"})), + ( + "a merge queue", + json!({"ref": "refs/heads/gh-readonly-queue/main/pr-7-abc", "event_name": "merge_group"}), + ), + ( + "workflow_dispatch", + json!({"event_name": "workflow_dispatch"}), + ), + ("a schedule", json!({"event_name": "schedule"})), + ("no event_name", json!({"event_name": null})), + ( + "another workflow", + json!({"job_workflow_ref": evil, "workflow_ref": evil}), + ), + ( + "a no workflow claim", + json!({"job_workflow_ref": null, "workflow_ref": null}), + ), + ]; + for (what, change) in cases { + let token = variant(&change); + for path in ["/v1/cache/upload", "/v1/cache/commit"] { + let status = write_status(&server, &token, path).await; + assert_eq!(status, 403, "{what} on {path}"); + } + } + // A repository off the allowlist is refused by the verifier itself. + let fork = token_with_claims("evil/fork", base.clone()); + assert_eq!(write_status(&server, &fork, "/v1/cache/upload").await, 403); + server.stop().await; +} + +#[tokio::test] +async fn test_results_uploads_need_the_ci_workflow_and_refuse_pull_request_target() { + let Some(server) = start().await else { + return; + }; + let upload = |extra: Value| { + let token = token_with_claims(REPO, extra); + let server = &server; + async move { write_status(server, &token, "/v1/test-results/runs").await } + }; + let base = json!({"ref": "refs/heads/main", "job_workflow_ref": CI_WORKFLOW, "repository_owner_id": "4242"}); + let merged = |change: Value| { + let mut claims = base.clone(); + for (name, value) in change.as_object().unwrap() { + claims[name] = value.clone(); + } + claims + }; + // Control: past the gate (an empty upload is the handler's 400, not a 403). + assert_eq!(upload(base.clone()).await, 400); + // Pull requests upload: a retry that passes there is the flake signal. + assert_eq!( + upload(merged( + json!({"ref": "refs/pull/7/merge", "event_name": "pull_request"}) + )) + .await, + 400 + ); + for (what, change) in [ + ( + "pull_request_target", + json!({"event_name": "pull_request_target"}), + ), + ( + "another workflow", + json!({"job_workflow_ref": "ArchAstro/firstlanding/.github/workflows/evil.yml@refs/heads/main"}), + ), + ("no workflow claim", json!({"job_workflow_ref": null})), + ] { + assert_eq!(upload(merged(change)).await, 403, "{what}"); + } + let fork = token_with_claims("evil/fork", base.clone()); + assert_eq!( + write_status(&server, &fork, "/v1/test-results/runs").await, + 403 + ); + server.stop().await; +} + +#[tokio::test] +async fn a_request_is_authenticated_before_its_body_is_read_and_capped_after() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + let Some(server) = start().await else { + return; + }; + let address = server.url.trim_start_matches("http://").to_string(); + // A declared 100 MB body that is never sent: if the server waited for it + // before checking the token, this would hang until the read timed out. + for path in [ + "/v1/cache/commit", + "/v1/cache/upload", + "/v1/cache/lookup", + "/v1/test-results/runs", + ] { + let mut stream = tokio::net::TcpStream::connect(&address).await.unwrap(); + stream + .write_all( + format!( + "POST {path} HTTP/1.1\r\nhost: x\r\ncontent-type: application/json\r\n\ + content-length: 100000000\r\n\r\n{{" + ) + .as_bytes(), + ) + .await + .unwrap(); + let mut reply = Vec::new(); + let read = tokio::time::timeout(std::time::Duration::from_secs(5), async { + let mut buffer = [0u8; 512]; + loop { + let n = stream.read(&mut buffer).await.unwrap(); + reply.extend_from_slice(&buffer[..n]); + if n == 0 || reply.windows(4).any(|w| w == b"\r\n\r\n") { + break; + } + } + }) + .await; + assert!(read.is_ok(), "{path}: no reply while the body was unsent"); + // Either refusal needs no body: 413 when the declared length is over + // the route's cap (checked from the header alone), else 401 from the + // gate. Never a wait for the body. + let reply_text = String::from_utf8_lossy(&reply); + assert!( + reply_text.starts_with("HTTP/1.1 401") || reply_text.starts_with("HTTP/1.1 413"), + "{path}: {}", + String::from_utf8_lossy(&reply) + ); + } + // With a valid token, a lookup or upload is capped at 1 MiB and a + // commit is not. + let ci = token_with_claims( + REPO, + json!({"repository_owner_id": "4242", "job_workflow_ref": CI_WORKFLOW, "ref": "refs/heads/main"}), + ); + let big = "x".repeat(2 << 20); + let client = aster_srv::http_client().build().unwrap(); + for (path, expected) in [ + ("/v1/cache/lookup", 413), + ("/v1/cache/upload", 413), + ("/v1/cache/commit", 400), + ] { + let status = client + .post(format!("{}{path}", server.url)) + .bearer_auth(&ci) + .body(big.clone()) + .send() + .await + .unwrap() + .status() + .as_u16(); + assert_eq!(status, expected, "{path}"); + } + server.stop().await; +} + +#[tokio::test] +async fn a_persons_lookup_does_not_count_or_keep_an_entry_alive_but_ci_does() { + let Some(server) = start().await else { + return; + }; + let ci = format!( + "Bearer {}", + token_with_claims( + REPO, + json!({"repository_owner_id": "4242", "job_workflow_ref": CI_WORKFLOW, "ref": "refs/heads/main"}), + ) + ); + let manifest = base64::engine::general_purpose::STANDARD + .encode(json!({"schema": 1, "key": "k", "target": "//a:b", "outputs": []}).to_string()); + let (status, body) = server + .post_json( + "/v1/cache/commit", + &json!({"repo": REPO, "key": "k", "manifest": manifest}), + Some(&ci), + None, + ) + .await; + assert_eq!(status, 201, "{body}"); + let hits = || async { + let row = server + .client() + .await + .query_one( + "SELECT hits, last_hit_at IS NOT NULL FROM cache.manifests WHERE key = 'k'", + &[], + ) + .await + .unwrap(); + (row.get::<_, i64>(0), row.get::<_, bool>(1)) + }; + let person = person_bearer(&server, "ada@example.com"); + let lookup = json!({"repo": REPO, "keys": ["k"]}); + for _ in 0..3 { + let (status, body) = server + .post_json("/v1/cache/lookup", &lookup, Some(&person), None) + .await; + assert_eq!(status, 200); + assert!(body["hits"]["k"].is_object(), "the person still reads it"); + } + assert_eq!( + hits().await, + (0, false), + "a person's lookups leave no trace" + ); + let (status, _) = server + .post_json("/v1/cache/lookup", &lookup, Some(&ci), None) + .await; + assert_eq!(status, 200); + assert_eq!(hits().await, (1, true), "CI's lookups count"); + server.stop().await; +} + +#[tokio::test] +async fn the_signed_blob_stand_in_exists_only_when_the_dev_flag_is_set() { + let Some(server) = start().await else { + return; + }; + let path = format!("/aster-cache-test/cas/{DIGEST}"); + // `start` leaves the flag at its test default, on. + let (on, _) = send(&server, "PUT", &path, &[]).await; + assert_ne!(on, 404, "the stand-in answers in the test setup"); + server.stop().await; + + let signer = Arc::new(FixedSigner { + email: "cache-test@example.iam.gserviceaccount.com".into(), + signature: b"test-signature".to_vec(), + signed: Default::default(), + }); + let store = BlobStore::memory("aster-cache-test", signer as Arc); + let Some(server) = common::start_with( + &[ + ("ASTER_SRV_CACHE_BUCKET", "aster-cache-test"), + ( + "ASTER_SRV_CACHE_SIGNER", + "cache-test@example.iam.gserviceaccount.com", + ), + ("ASTER_SRV_DEV_BLOB_ROUTE", "0"), + ], + Some(store), + ) + .await + else { + return; + }; + for method in ["PUT", "GET"] { + let (status, _) = send(&server, method, &path, &[]).await; + assert_eq!( + status, 404, + "{method} without the flag, even on a memory store" + ); + } + server.stop().await; +} diff --git a/docs/test-tracking.md b/docs/test-tracking.md index fff5d48a..0801d6ae 100644 --- a/docs/test-tracking.md +++ b/docs/test-tracking.md @@ -152,10 +152,31 @@ Signed out, or with an expired or refused token, `aster run` prints one line does not fail. If the server has not enabled sign-in, `aster cache login` says the server refused it. -A person's token reads the cache and nothing else: it cannot upload or commit -(the server answers `403`), cannot use test-result routes, and Aster never -asks to write with it. Entries CI writes carry the CI runner's platform in -their key, so a macOS laptop mostly misses entries written on Linux CI. +A person's token reads and never writes: it cannot upload or commit (the +server answers `403`), cannot upload test results, and Aster never asks to +write with it. Entries CI writes carry the CI runner's platform in their key, +so a macOS laptop mostly misses entries written on Linux CI. + +### Reading the dashboard's data + +The same token reads every dataset the `/ui` pages show, as JSON, for any +repository the server serves. `aster srv` prints it; the repository defaults to +`GITHUB_REPOSITORY`, else the checkout's `origin` (`--repo owner/name` +overrides). + +| `/ui` page | Command | Route | +|---|---|---| +| Cache | `aster srv cache` | `GET /v1/cache/summary`, `/v1/cache/entries?target=` | +| Flaky tests | `aster srv flaky` | `GET /v1/test-results/tests?state=flaky` | +| Flaky tests (broken list) | `aster srv broken` | `GET /v1/test-results/tests?state=broken` | +| Failure clusters | `aster srv clusters` | `GET /v1/test-results/clusters` | +| Quarantine | `aster srv quarantine` | `GET /v1/test-results/quarantine` | +| A test | `aster srv test ` | `GET /v1/test-results/tests/` | +| Anything else | `aster srv get ''` | any `GET /v1/...` route, such as `/v1/test-results/durations?...` or `/v1/test-results/runs//failures` | + +With CI credentials in the environment (`ASTER_SRV_TOKEN`, GitHub OIDC) the CI +token is used and reads its own repository. Not signed in, or a refused token, +exits non-zero with the reason. ## 4. Flaky and broken diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 56415716..dc9f9399 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -35,7 +35,9 @@ pub use gha::{client::GhaCacheClient, CacheSummary, GhaBackend}; pub use hasher::{CacheHasher, DependencyHash, EnvInput, HashedFile, KeyMaterial, RuntimeInput}; pub use http::{HttpCache, REMOTE_CACHE_TOKEN_ENV, REMOTE_CACHE_URL_ENV}; pub use local::{LocalStore, PruneStats}; -pub use login::{login, LoginOptions, SignedIn, LOGIN_TIMEOUT, SIGNED_OUT_MESSAGE}; +pub use login::{ + login, stored_token, LoginOptions, SignedIn, StoredToken, LOGIN_TIMEOUT, SIGNED_OUT_MESSAGE, +}; pub use matcher::TargetInputMatcher; pub use outputs::{pack, restore}; pub use prune::{prune_cache, prune_if_over_cap, PruneLimits, DEFAULT_MAX_AGE, DEFAULT_MAX_SIZE}; diff --git a/src/cli/commands.rs b/src/cli/commands.rs index 6d779b33..83149a7e 100644 --- a/src/cli/commands.rs +++ b/src/cli/commands.rs @@ -354,6 +354,17 @@ pub enum Commands { command: ResultsCommands, }, + /// Read aster-srv's dashboard data as JSON + /// + /// Prints what the dashboard pages show, from the server's read-only + /// `/v1` routes. Sign in once with `aster cache login`; CI credentials + /// (`ASTER_SRV_TOKEN`, GitHub OIDC) are used when present. Exits non-zero + /// with a message when you are not signed in or the server refuses. + Srv { + #[command(subcommand)] + command: SrvCommands, + }, + /// Watch targets and rerun them when their declared inputs change /// /// Pass one or more target addresses (//project:target) or bare projects @@ -602,6 +613,64 @@ pub enum CacheCommands { }, } +/// Which server and repository `aster srv` reads. +#[derive(clap::Args, Clone)] +pub struct SrvTarget { + /// The aster-srv URL [default: $ASTER_SRV_URL] + #[arg(long)] + pub url: Option, + /// The repository, owner/name [default: $GITHUB_REPOSITORY, else the checkout's origin] + #[arg(long)] + pub repo: Option, +} + +/// `aster srv` subcommands +#[derive(Subcommand)] +pub enum SrvCommands { + /// GET any aster-srv path and print the JSON + /// + /// For example `aster srv get '/v1/test-results/tests?state=flaky&limit=20'`. + /// A `/v1/` path that names no `repo` gets the default one. + Get { + /// The path and query, starting with / + path: String, + #[command(flatten)] + target: SrvTarget, + }, + /// The cache's size, lookups and last writer (the Cache page) + Cache { + #[command(flatten)] + target: SrvTarget, + }, + /// Flaky tests, highest score first (the Flaky tests page) + Flaky { + #[command(flatten)] + target: SrvTarget, + }, + /// Broken tests (the Flaky tests page's broken list) + Broken { + #[command(flatten)] + target: SrvTarget, + }, + /// The quarantine list (the Quarantine page) + Quarantine { + #[command(flatten)] + target: SrvTarget, + }, + /// The last week's failures grouped by failure hash (the Failure clusters page) + Clusters { + #[command(flatten)] + target: SrvTarget, + }, + /// One test's state and recent results (a test's page) + Test { + /// The test id, as listed by `aster srv flaky` + id: String, + #[command(flatten)] + target: SrvTarget, + }, +} + /// Test results subcommands #[derive(Subcommand)] pub enum ResultsCommands { diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 56a9bac2..b32686aa 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -12,7 +12,7 @@ pub mod source_graph; pub use commands::{ CacheCommands, CiCommands, Cli, Commands, ProjectCommands, ResultsCommands, ServicesCommands, - ServicesDaemonCommands, TlsCommands, + ServicesDaemonCommands, SrvCommands, TlsCommands, }; pub use output::{ build_execution_output, build_execution_output_with_cache, output_json, print_summary, diff --git a/src/main.rs b/src/main.rs index ed4db322..ad743d9c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,7 +18,7 @@ use aster::cli::{ build_execution_output, build_execution_output_with_cache, check_reserved_target, expand_selection, output_json, parse_run_args, print_summary, select_projects, CacheCommands, CiCommands, Cli, Commands, GraphOutput, OutputMode, ProjectCommands, ProjectInfo, - ResultsCommands, ServicesCommands, ServicesDaemonCommands, TlsCommands, WhyOutput, + ResultsCommands, ServicesCommands, ServicesDaemonCommands, SrvCommands, TlsCommands, WhyOutput, SKILLS_MARKDOWN, }; use aster::config::{find_workspace_root, WorkspaceConfig}; @@ -99,6 +99,9 @@ fn run() -> Result<()> { if let Commands::Results { command } = &command { return handle_results(command, output_mode); } + if let Commands::Srv { command } = &command { + return handle_srv(command); + } if let Commands::Services { command: ServicesCommands::Daemon { command }, } = &command @@ -342,6 +345,7 @@ fn run() -> Result<()> { match command { Commands::Init => unreachable!("Init handled above"), Commands::Results { .. } => unreachable!("results handled above"), + Commands::Srv { .. } => unreachable!("srv handled above"), Commands::List { path, lang } => { validate_lang_filter(&lang)?; @@ -1853,6 +1857,39 @@ fn handle_results(command: &ResultsCommands, output_mode: OutputMode) -> Result< } } +/// `aster srv ...`: print one dashboard dataset as JSON. +fn handle_srv(command: &SrvCommands) -> Result<()> { + use aster::test_results::read::{self, Dataset}; + let (path, target) = match command { + SrvCommands::Get { path, target } => { + if !path.starts_with('/') { + anyhow::bail!("the path must start with /, for example /v1/cache/summary"); + } + (path.clone(), target) + } + SrvCommands::Cache { target } => (Dataset::CacheSummary.path(), target), + SrvCommands::Flaky { target } => (Dataset::Flaky.path(), target), + SrvCommands::Broken { target } => (Dataset::Broken.path(), target), + SrvCommands::Quarantine { target } => (Dataset::Quarantine.path(), target), + SrvCommands::Clusters { target } => (Dataset::Clusters.path(), target), + SrvCommands::Test { id, target } => (Dataset::Test(id.clone()).path(), target), + }; + let url = srv_url(target.url.as_deref()) + .ok_or_else(|| anyhow::anyhow!("give --url or set ASTER_SRV_URL to your aster-srv URL"))?; + let cwd = env::current_dir().context("Failed to get current directory")?; + let root = find_workspace_root(&cwd).unwrap_or(cwd); + let env_var = |name: &str| env::var(name).ok(); + let repo = read::default_repo(target.repo.as_deref(), &root, &env_var); + let body = read::get( + &url, + &read::with_repo(&path, repo.as_deref()), + &env_var, + std::time::SystemTime::now(), + )?; + println!("{}", serde_json::to_string_pretty(&body)?); + Ok(()) +} + /// The quarantine for a run, loaded once per invocation (a run over /// several lanes executes several times). Only a run that captures test /// results can use one. diff --git a/src/test_results/client.rs b/src/test_results/client.rs index beb4ae97..5e9dba54 100644 --- a/src/test_results/client.rs +++ b/src/test_results/client.rs @@ -81,6 +81,42 @@ impl Server { .context("cannot read the server's reply") } + /// A server that sends a signed-in person's token (`aster cache login`) + /// instead of a CI credential. + pub fn with_token( + url: &str, + token: String, + proxy_token: Option, + timeout: Duration, + ) -> Self { + Self { + agent: agent(timeout), + base: url.trim_end_matches('/').to_string(), + token, + proxy_token, + } + } + + /// `GET path` (with its query): the status and the JSON body, `Null` + /// when the body is not JSON. Any status is an answer; only a network + /// failure is an error. + pub fn get_status(&self, path: &str) -> Result<(u16, Value)> { + let url = format!("{}{path}", self.base); + let mut request = self + .agent + .get(&url) + .header("Authorization", &format!("Bearer {}", self.token)); + if let Some(proxy) = &self.proxy_token { + request = request.header("Proxy-Authorization", &format!("Bearer {proxy}")); + } + let mut response = request + .call() + .with_context(|| format!("cannot reach {}", self.base))?; + let code = response.status().as_u16(); + let body = response.body_mut().read_json().unwrap_or(Value::Null); + Ok((code, body)) + } + /// The server's record of one test, or `None` when it does not know it. pub fn test(&self, test_id: &str) -> Result> { self.get_json(&format!( diff --git a/src/test_results/mod.rs b/src/test_results/mod.rs index dbd8a40c..9e349ed0 100644 --- a/src/test_results/mod.rs +++ b/src/test_results/mod.rs @@ -19,6 +19,7 @@ mod junit; mod libtest; pub(crate) mod nodetest; pub mod quarantine; +pub mod read; pub(crate) mod stream; pub mod summary; pub mod upload; diff --git a/src/test_results/read.rs b/src/test_results/read.rs new file mode 100644 index 00000000..d7dd4be7 --- /dev/null +++ b/src/test_results/read.rs @@ -0,0 +1,174 @@ +//! `aster srv`: read the datasets aster-srv's dashboard shows, as JSON. +//! +//! Each dataset is one `GET /v1/...` route. The credential is the CI one +//! when the environment has it (`ASTER_SRV_TOKEN`, GitHub OIDC), else the +//! token `aster cache login` stored for this server. A person's token can +//! read every dataset of any repository the server serves; it names the +//! repository with `repo`, which defaults the way the cache client derives +//! it (`GITHUB_REPOSITORY`, else the checkout's `origin`). + +use std::path::Path; +use std::time::{Duration, SystemTime}; + +use anyhow::{anyhow, bail, Result}; +use serde_json::Value; + +use super::client::{percent_encode, Server}; +use super::upload::{proxy_token_from_env, Credentials}; +use crate::cache::{stored_token, SIGNED_OUT_MESSAGE}; + +const TIMEOUT: Duration = Duration::from_secs(20); + +/// A dashboard dataset with its own subcommand. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Dataset { + /// Cache size, lookups and last writer. + CacheSummary, + /// Tests in the flaky state, highest score first. + Flaky, + /// Tests that fail on every default-branch run since some commit. + Broken, + /// The tests Aster keeps from failing builds. + Quarantine, + /// The last week's failures grouped by failure hash. + Clusters, + /// One test's state and recent results. + Test(String), +} + +impl Dataset { + pub fn path(&self) -> String { + match self { + Dataset::CacheSummary => "/v1/cache/summary".into(), + Dataset::Flaky => "/v1/test-results/tests?state=flaky".into(), + Dataset::Broken => "/v1/test-results/tests?state=broken".into(), + Dataset::Quarantine => "/v1/test-results/quarantine".into(), + Dataset::Clusters => "/v1/test-results/clusters".into(), + Dataset::Test(id) => format!("/v1/test-results/tests/{}", percent_encode(id)), + } + } +} + +/// `path` with `repo` added when it is a `/v1/` route that names none. +pub fn with_repo(path: &str, repo: Option<&str>) -> String { + let Some(repo) = repo else { + return path.to_string(); + }; + let (route, query) = path.split_once('?').unwrap_or((path, "")); + let names_repo = query + .split('&') + .any(|pair| pair.split('=').next() == Some("repo")); + if !route.starts_with("/v1/") || names_repo { + return path.to_string(); + } + let separator = if query.is_empty() { '?' } else { '&' }; + format!("{path}{separator}repo={}", percent_encode(repo)) +} + +/// The repository to read: `--repo`, else `GITHUB_REPOSITORY` or the +/// checkout's `origin`. +pub fn default_repo( + flag: Option<&str>, + workspace_root: &Path, + env: &dyn Fn(&str) -> Option, +) -> Option { + flag.map(str::to_string) + .or_else(|| Some(super::detect_run(workspace_root, "", None, env).repo)) + .filter(|repo| !repo.trim().is_empty()) +} + +/// `GET path` on `url` and return the body, or say why not. +pub fn get( + url: &str, + path: &str, + env: &dyn Fn(&str) -> Option, + now: SystemTime, +) -> Result { + let proxy_token = proxy_token_from_env(env); + let (server, person) = match Credentials::from_env(env) { + Some(credentials) => (Server::connect(url, &credentials, proxy_token)?, false), + None => { + let stored = stored_token(url, now).ok_or_else(|| anyhow!(SIGNED_OUT_MESSAGE))?; + ( + Server::with_token(url, stored.token, proxy_token, TIMEOUT), + true, + ) + } + }; + let (status, body) = server.get_status(path)?; + if (200..300).contains(&status) { + return Ok(body); + } + let reason = body["error"].as_str().unwrap_or("no reason given"); + match status { + 401 if person => { + bail!("the server did not accept your sign-in ({reason}); run aster cache login") + } + 401 => bail!("the server did not accept the credentials ({reason})"), + 403 => bail!("the server refused the request (403): {reason}"), + 404 => bail!("not found: {path}"), + other => bail!("the server answered {other}: {reason}"), + } +} + +#[cfg(test)] +mod tests { + use super::super::client::testing::serve; + use super::*; + + #[test] + fn datasets_map_to_their_routes() { + assert_eq!(Dataset::Flaky.path(), "/v1/test-results/tests?state=flaky"); + assert_eq!( + Dataset::Test("a/b c".into()).path(), + "/v1/test-results/tests/a%2Fb%20c" + ); + } + + #[test] + fn repo_is_added_to_v1_routes_that_name_none() { + let repo = Some("acme/app"); + assert_eq!( + with_repo("/v1/test-results/quarantine", repo), + "/v1/test-results/quarantine?repo=acme%2Fapp" + ); + assert_eq!( + with_repo("/v1/test-results/tests?state=flaky", repo), + "/v1/test-results/tests?state=flaky&repo=acme%2Fapp" + ); + assert_eq!( + with_repo("/v1/cache/summary?repo=other/x", repo), + "/v1/cache/summary?repo=other/x" + ); + assert_eq!(with_repo("/healthz", repo), "/healthz"); + assert_eq!(with_repo("/v1/cache/summary", None), "/v1/cache/summary"); + } + + #[test] + fn a_ci_token_is_sent_and_a_refusal_is_explained() { + let (url, seen) = serve(vec![ + ("/v1/ok", 200, r#"{"tests":[]}"#.into()), + ("/v1/denied", 403, r#"{"error":"nope"}"#.into()), + ("/v1/stale", 401, r#"{"error":"expired"}"#.into()), + ]); + let env = |name: &str| (name == "ASTER_SRV_TOKEN").then(|| "ci-secret".to_string()); + let now = SystemTime::now(); + assert_eq!( + get(&url, "/v1/ok", &env, now).unwrap()["tests"], + serde_json::json!([]) + ); + assert!(seen.lock().unwrap()[0].contains("authorization: bearer ci-secret")); + let denied = get(&url, "/v1/denied", &env, now).unwrap_err().to_string(); + assert!( + denied.contains("403") && denied.contains("nope"), + "{denied}" + ); + let stale = get(&url, "/v1/stale", &env, now).unwrap_err().to_string(); + assert!( + stale.contains("did not accept") && stale.contains("expired"), + "{stale}" + ); + let missing = get(&url, "/v1/absent", &env, now).unwrap_err().to_string(); + assert!(missing.contains("not found"), "{missing}"); + } +} diff --git a/tests/cache_login_e2e.rs b/tests/cache_login_e2e.rs index 026464dd..18c3709c 100644 --- a/tests/cache_login_e2e.rs +++ b/tests/cache_login_e2e.rs @@ -32,7 +32,7 @@ const OIDC_PUBLIC: &[u8] = include_bytes!("fixtures/test-oidc-key.pub.pem"); const REPO: &str = "acme/app"; const PERSON: &str = "ada@example.com"; const CI_WORKFLOW: &str = "acme/app/.github/workflows/ci.yml@refs/heads/main"; -const IAP_AUDIENCE: &str = "/projects/123456/global/backendServices/"; +const IAP_AUDIENCE: &str = "/projects/123456/global/backendServices/4242"; const CLI_KEY: &str = "e2e1:Rm7vKq2LxT9bYc4nWz8PdJ3hSeA6UgF1oIiNtXuMvB0"; struct Running { @@ -72,7 +72,7 @@ impl Running { &header, &json!({ "iss": "https://cloud.google.com/iap", - "aud": format!("{IAP_AUDIENCE}4242"), + "aud": IAP_AUDIENCE, "exp": chrono::Utc::now().timestamp() + 300, "email": email, "sub": "accounts.google.com:e2e-1001", @@ -88,9 +88,11 @@ async fn start() -> Option { eprintln!("skipping: ASTER_SRV_TEST_DATABASE_URL is not set"); return None; }; + static NEXT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); let database = format!( - "aster_login_e2e_{}_{}", + "aster_login_e2e_{}_{}_{}", std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed), std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .expect("clock") @@ -118,6 +120,7 @@ async fn start() -> Option { "ASTER_SRV_CACHE_BUCKET" => Some("aster-cache-e2e".into()), "ASTER_SRV_CACHE_SIGNER" => Some("cache-e2e@example.iam.gserviceaccount.com".into()), "ASTER_SRV_IAP_AUDIENCE" => Some(IAP_AUDIENCE.into()), + "ASTER_SRV_DEV_BLOB_ROUTE" => Some("1".into()), "ASTER_SRV_UI_EMAIL_DOMAINS" => Some("example.com".into()), "ASTER_SRV_PUBLIC_URL" => Some(url.clone()), // The e2e server listens on loopback http. @@ -179,6 +182,7 @@ fn github_oidc_endpoint(stop: Arc) -> String { "ref": "refs/heads/main", "sha": "0123abc", "run_id": "7001", + "event_name": "push", "workflow_ref": CI_WORKFLOW, "job_workflow_ref": CI_WORKFLOW, }), @@ -371,6 +375,58 @@ fn login_with( (status, reader.join().unwrap()) } +/// `aster cache login` against the real server, with the scripted browser +/// playing a person behind IAP: GET the page, confirm, and follow the +/// server's redirect to the CLI's loopback listener. +async fn sign_in( + server: &Running, + dir: &Path, + config_home: &Path, +) -> (std::process::ExitStatus, Vec) { + let url = server.url.clone(); + let assertion = server.iap_assertion(PERSON); + tokio::task::spawn_blocking({ + let (dir, url, config) = (dir.to_path_buf(), url, config_home.to_path_buf()); + move || { + login_with(&dir, &url, &config, |sign_in_url| { + let agent = browser(); + // The confirmation page names who is signing in. + let page = reply( + agent + .get(sign_in_url) + .header("x-goog-iap-jwt-assertion", &assertion) + .call() + .expect("GET /cli-auth"), + ); + assert_eq!(page.status, 200, "{}", page.body); + assert!(page.body.contains(PERSON), "{}", page.body); + // The person clicks Authorize. + let confirmed = reply( + agent + .post(format!("{url}/cli-auth")) + .header("content-type", "application/x-www-form-urlencoded") + .header("x-goog-iap-jwt-assertion", &assertion) + .send(form_body(&page.body)) + .expect("POST /cli-auth"), + ); + assert_eq!(confirmed.status, 302, "{}", confirmed.body); + let callback = confirmed.location.expect("a redirect"); + assert!( + callback.starts_with("http://127.0.0.1:") + && callback.contains("/callback?code="), + "the redirect goes to the loopback listener only: {callback}" + ); + // The browser lands on the CLI's listener, with no IAP header. + let landed = reply(agent.get(&callback).call().expect("GET /callback")); + assert_eq!(landed.status, 200, "{}", landed.body); + assert!(landed.body.contains("close this tab"), "{}", landed.body); + }) + } + }) + .await + .unwrap() +} + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn a_laptop_signs_in_through_aster_srv_and_restores_what_ci_wrote() { let Some(server) = start().await else { @@ -443,47 +499,7 @@ async fn a_laptop_signs_in_through_aster_srv_and_restores_what_ci_wrote() { // Phase 3: `aster cache login` against the real server. The scripted // browser is a person behind IAP: GET the page, confirm, and follow the // server's redirect to the CLI's loopback listener. - let assertion = server.iap_assertion(PERSON); - let (status, lines) = tokio::task::spawn_blocking({ - let (dir, url, config) = (laptop.clone(), url.clone(), config_home.clone()); - move || { - login_with(&dir, &url, &config, |sign_in_url| { - let agent = browser(); - // The confirmation page names who is signing in. - let page = reply( - agent - .get(sign_in_url) - .header("x-goog-iap-jwt-assertion", &assertion) - .call() - .expect("GET /cli-auth"), - ); - assert_eq!(page.status, 200, "{}", page.body); - assert!(page.body.contains(PERSON), "{}", page.body); - // The person clicks Authorize. - let confirmed = reply( - agent - .post(format!("{url}/cli-auth")) - .header("content-type", "application/x-www-form-urlencoded") - .header("x-goog-iap-jwt-assertion", &assertion) - .send(form_body(&page.body)) - .expect("POST /cli-auth"), - ); - assert_eq!(confirmed.status, 302, "{}", confirmed.body); - let callback = confirmed.location.expect("a redirect"); - assert!( - callback.starts_with("http://127.0.0.1:") - && callback.contains("/callback?code="), - "the redirect goes to the loopback listener only: {callback}" - ); - // The browser lands on the CLI's listener, with no IAP header. - let landed = reply(agent.get(&callback).call().expect("GET /callback")); - assert_eq!(landed.status, 200, "{}", landed.body); - assert!(landed.body.contains("close this tab"), "{}", landed.body); - }) - } - }) - .await - .unwrap(); + let (status, lines) = sign_in(&server, &laptop, &config_home).await; let transcript = lines.join("\n"); assert!(status.success(), "login failed: {transcript}"); assert!( @@ -562,3 +578,233 @@ async fn a_laptop_signs_in_through_aster_srv_and_restores_what_ci_wrote() { server.stop().await; } + +/// What `aster srv ` printed and how it exited. +async fn srv(dir: &Path, url: &str, config_home: &Path, args: &[&str]) -> (bool, Value, String) { + let output = tokio::task::spawn_blocking({ + let (dir, url, config) = ( + dir.to_path_buf(), + url.to_string(), + config_home.to_path_buf(), + ); + let args: Vec = args.iter().map(|arg| arg.to_string()).collect(); + move || { + aster(&dir) + .arg("srv") + .args(args) + .env("ASTER_SRV_URL", url) + .env("XDG_CONFIG_HOME", config) + .output() + .expect("spawn aster srv") + } + }) + .await + .unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + ( + output.status.success(), + serde_json::from_str(&stdout).unwrap_or(Value::Null), + String::from_utf8_lossy(&output.stderr).into_owned(), + ) +} + +/// One CI run of `//app:test`, as `--test-results` writes it: `flaky_test` +/// fails and then passes on retry, `broken_test` fails on every attempt. +fn ci_results(run: usize) -> String { + let record = |value: Value| format!("{value}\n"); + let test = |id: &str, outcome: &str, attempt: u32, hash: Option<&str>| { + record(json!({ + "type": "test", "target": "//app:test", "test_id": id, "file": "app/t.exs", + "name": id, "outcome": outcome, "attempt": attempt, "duration_ms": 5, + "failure_hash": hash, "message": hash.map(|_| format!("boom in {id}")), + })) + }; + [ + record(json!({ + "type": "run", "run_id": format!("e2e-run-{run}"), "repo": REPO, + "sha": format!("{run:040}"), "branch": "main", "aster_version": "0.0.0", + "started_at": chrono::Utc::now().to_rfc3339(), + })), + record(json!({ + "type": "target", "address": "//app:test", "input_hash": format!("h{run}"), + "status": "failed", "cached": false, "duration_ms": 10, "confidence": "complete", + })), + test("flaky_test", "fail", 1, Some("flake-hash")), + test("flaky_test", "pass", 2, None), + test("broken_test", "fail", 1, Some("broken-hash")), + ] + .concat() +} + +/// A person who signed in with `aster cache login` can read every dataset the +/// dashboard shows through `aster srv`, from data CI uploaded, and the server +/// refuses their writes. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_signed_in_laptop_reads_every_dashboard_dataset_with_aster_srv() { + let Some(server) = start().await else { + return; + }; + let root = tempfile::tempdir().unwrap(); + let ci_checkout = root.path().join("ci").join("workspace"); + let laptop = root.path().join("laptop").join("workspace"); + let config_home = root.path().join("laptop-config"); + for dir in [&ci_checkout, &laptop] { + checkout(dir); + } + let url = server.url.clone(); + + // Phase 1: CI uploads three runs of a suite with a flaky test and a broken + // one, with the job's OIDC token, through `aster results upload`. + let stop_oidc = Arc::new(AtomicBool::new(false)); + let oidc_url = github_oidc_endpoint(stop_oidc.clone()); + for run in 1..=3 { + let file = root.path().join(format!("results-{run}.ndjson")); + fs::write(&file, ci_results(run)).unwrap(); + let uploaded = tokio::task::spawn_blocking({ + let (dir, url, oidc_url) = (ci_checkout.clone(), url.clone(), oidc_url.clone()); + move || { + aster(&dir) + .args(["results", "upload", "--strict"]) + .arg(&file) + .env("ASTER_SRV_URL", &url) + .env("ACTIONS_ID_TOKEN_REQUEST_URL", oidc_url) + .env("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "request-token") + .output() + .expect("spawn aster results upload") + } + }) + .await + .unwrap(); + assert!( + uploaded.status.success(), + "upload {run}: {}", + String::from_utf8_lossy(&uploaded.stderr) + ); + } + stop_oidc.store(true, Ordering::Relaxed); + // The server's classifier decides flaky, broken and quarantined; run its + // pass now instead of waiting for the schedule. + aster_srv::test_results::classifier::run_once( + &server.app, + chrono::Utc::now() + chrono::Duration::hours(1), + ) + .await + .unwrap() + .expect("the classifier lock is free"); + + // Phase 2: before signing in, the laptop is told how, and prints no JSON. + let (ok, body, stderr) = srv(&laptop, &url, &config_home, &["flaky"]).await; + assert!(!ok && body.is_null(), "{body}"); + assert!( + stderr.contains("not signed in; run aster cache login"), + "{stderr}" + ); + + // Phase 3: sign in, as in the cache test. + let (status, lines) = sign_in(&server, &laptop, &config_home).await; + assert!(status.success(), "login failed: {}", lines.join("\n")); + + // Phase 4: one read per dataset. The repository is the checkout's + // (GITHUB_REPOSITORY), which the person never typed. + let today = chrono::Utc::now().date_naive().to_string(); + let durations = format!("/v1/test-results/durations?target=%2F%2Fapp%3Atest&day={today}"); + let ids = |body: &Value, key: &str| -> Vec { + body[key] + .as_array() + .unwrap_or(&Vec::new()) + .iter() + .map(|entry| entry["test_id"].as_str().unwrap_or_default().to_string()) + .collect() + }; + let read = |args: &'static [&'static str]| { + let (laptop, url, config_home) = (laptop.clone(), url.clone(), config_home.clone()); + async move { + let (ok, body, stderr) = srv(&laptop, &url, &config_home, args).await; + assert!(ok, "aster srv {args:?}: {stderr}"); + assert_eq!(body["repo"], REPO, "aster srv {args:?}: {body}"); + body + } + }; + assert!(read(&["cache"]).await["entries"].is_null()); + assert_eq!(ids(&read(&["flaky"]).await, "tests"), ["flaky_test"]); + assert_eq!(ids(&read(&["broken"]).await, "tests"), ["broken_test"]); + let quarantine = read(&["quarantine"]).await; + assert_eq!(ids(&quarantine, "tests"), ["flaky_test"], "{quarantine}"); + let clusters = read(&["clusters"]).await; + let hashes: Vec<&str> = clusters["clusters"] + .as_array() + .unwrap() + .iter() + .map(|cluster| cluster["failure_hash"].as_str().unwrap()) + .collect(); + assert_eq!(hashes.len(), 2, "{clusters}"); + assert!(hashes.contains(&"flake-hash") && hashes.contains(&"broken-hash")); + let history = read(&["test", "flaky_test"]).await; + assert_eq!(history["test"]["state"], "flaky", "{history}"); + assert_eq!(history["history"].as_array().unwrap().len(), 6, "{history}"); + let by_path = { + let (ok, body, stderr) = srv( + &laptop, + &url, + &config_home, + &["get", "/v1/test-results/runs/e2e-run-1/failures"], + ) + .await; + assert!(ok, "{stderr}"); + body + }; + assert_eq!(by_path["run"]["repo"], REPO, "{by_path}"); + let (ok, durations_body, stderr) = srv(&laptop, &url, &config_home, &["get", &durations]).await; + assert!(ok, "{stderr}"); + assert!(durations_body["files"].is_array(), "{durations_body}"); + + // Phase 5: a repository the server does not serve, and writes, are 403. + let (ok, body, stderr) = srv( + &laptop, + &url, + &config_home, + &["flaky", "--repo", "evil/other"], + ) + .await; + assert!(!ok && body.is_null(), "{body}"); + assert!(stderr.contains("403"), "{stderr}"); + let stored: Value = serde_json::from_str( + &fs::read_to_string(config_home.join("aster/cache-tokens").join(format!( + "{}.json", + url.trim_start_matches("http://").replace(':', "_") + ))) + .unwrap(), + ) + .unwrap(); + let token = stored["token"].as_str().unwrap().to_string(); + for (path, body) in [ + ("/v1/test-results/runs", "{}".to_string()), + ( + "/v1/cache/upload", + json!({"repo": REPO, "digests": ["0".repeat(64)]}).to_string(), + ), + ( + "/v1/cache/commit", + json!({"repo": REPO, "key": "k", "manifest": ""}).to_string(), + ), + ] { + let write = tokio::task::spawn_blocking({ + let (url, token) = (url.clone(), token.clone()); + move || { + reply( + browser() + .post(format!("{url}{path}")) + .header("authorization", format!("Bearer {token}")) + .header("content-type", "application/json") + .send(body) + .expect("POST"), + ) + } + }) + .await + .unwrap(); + assert_eq!(write.status, 403, "POST {path}: {}", write.body); + } + + server.stop().await; +} diff --git a/tests/cache_remote_e2e.rs b/tests/cache_remote_e2e.rs index 479b7eb6..b5d37c92 100644 --- a/tests/cache_remote_e2e.rs +++ b/tests/cache_remote_e2e.rs @@ -79,6 +79,7 @@ async fn start() -> Option { "ASTER_SRV_DATABASE_URL" => Some(database_url.clone()), "ASTER_SRV_REPOSITORIES" => Some("acme/*".into()), "ASTER_SRV_CACHE_BUCKET" => Some("aster-cache-e2e".into()), + "ASTER_SRV_DEV_BLOB_ROUTE" => Some("1".into()), "ASTER_SRV_CACHE_SIGNER" => Some("cache-e2e@example.iam.gserviceaccount.com".into()), "ASTER_SRV_STATIC_TOKENS" => Some(format!("{REPO}={TOKEN}")), _ => None,