From dc2858e02cf26b124b5ed0ee93bca3e11ab283a2 Mon Sep 17 00:00:00 2001 From: Itamar Zand Date: Sun, 24 May 2026 01:03:09 +0300 Subject: [PATCH 1/6] feat(manager): per-project scope on Subject, enforced on OCI push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `pub scopes: Vec` to `Subject` so per-project authorization tuples (format `"/"`) can flow from a token validator that knows about them down to the OCI push handler. Default- empty so the field is invisible to validators that don't populate it (`TokenDbValidator`, `PermissiveAuthValidator`) — OSS standalone behaviour is preserved bit-for-bit. `require_push_auth` in the registry proxy gains a scope check: after the existing role-based `can_push_image` gate passes, if `subject.scopes` is non-empty, at least one entry must end with `/` (suffix-match rather than full tuple — project IDs are globally-unique ULIDs, and the caller's scope carries their workspace_id while `subject.workspace_id` can be a different workspace's id when the call is routed through a shared manager). Empty `subject.scopes` short-circuits the check, again preserving OSS behaviour. `upstream_repository_prefix()` on `LocalArtifactRegistry` returned a hardcoded `"artifacts/default"` — that was always wrong but masked because the proxy routing table never had to extract a real project_id when nothing did per-project authz. With the scope check above, a wrong prefix breaks routing in a way that's now observable. Return `self.binding_name.clone()` (i.e. just `"artifacts"`) so the prefix matches the actual on-disk layout and the routing table extracts the real project_id from `artifacts//...`. Root-cause fix. Test fixtures: every Subject literal in oss_authz / permissive_auth / token_db_validator and the three tests/* files gets `scopes: Vec::new()` — pure field-threading, no behaviour change. --- .../src/providers/artifact_registry/local.rs | 14 ++++++---- crates/alien-manager/src/auth/subject.rs | 9 +++++++ .../alien-manager/src/providers/oss_authz.rs | 3 +++ .../src/providers/permissive_auth.rs | 1 + .../src/providers/token_db_validator.rs | 1 + .../src/routes/registry_proxy.rs | 26 +++++++++++++++---- .../tests/registry_proxy_cloud_test.rs | 1 + .../tests/registry_proxy_test.rs | 1 + .../alien-manager/tests/sqlite_store_tests.rs | 1 + 9 files changed, 47 insertions(+), 10 deletions(-) diff --git a/crates/alien-bindings/src/providers/artifact_registry/local.rs b/crates/alien-bindings/src/providers/artifact_registry/local.rs index 1f655da2..58138a14 100644 --- a/crates/alien-bindings/src/providers/artifact_registry/local.rs +++ b/crates/alien-bindings/src/providers/artifact_registry/local.rs @@ -147,11 +147,15 @@ impl ArtifactRegistry for LocalArtifactRegistry { } fn upstream_repository_prefix(&self) -> String { - // The embedded local registry accepts two-segment repo paths (e.g., - // "namespace/repo"). We use "artifacts/default" as the canonical prefix - // — this matches what the CLI hardcodes in dev mode and what the proxy - // routing table uses to route pushes to this local registry. - "artifacts/default".to_string() + // The binding name is the routing prefix; the next path segment is the + // project namespace (`artifacts/{project_id}` in platform mode, + // `artifacts/default` in dev mode). Returning the bare binding name + // here — instead of the legacy `"artifacts/default"` — lets the proxy + // routing table correctly identify both modes' pushes as local and + // extract the right project_id for downstream authz/scope checks. + // Matches the per-cloud convention documented in + // `internal-docs/alien/02-manager/02-releases.md`. + self.binding_name.clone() } async fn create_repository(&self, repo_name: &str) -> Result { diff --git a/crates/alien-manager/src/auth/subject.rs b/crates/alien-manager/src/auth/subject.rs index 34ad61e4..13780264 100644 --- a/crates/alien-manager/src/auth/subject.rs +++ b/crates/alien-manager/src/auth/subject.rs @@ -25,6 +25,11 @@ pub struct Subject { pub workspace_id: String, pub scope: Scope, pub role: Role, + /// Per-project scopes from the credential. Format: `"${workspace_id}/${project_id}"`. + /// Empty means no narrowing — OSS validators (`TokenDbValidator`, + /// `PermissiveAuthValidator`) leave this empty and `require_push_auth` + /// falls back to the role/scope check, preserving OSS standalone behavior. + pub scopes: Vec, /// The raw bearer token the caller presented. /// /// Lifecycle: request-scoped only. Constructed by the AuthValidator from @@ -47,6 +52,7 @@ impl fmt::Debug for Subject { .field("workspace_id", &self.workspace_id) .field("scope", &self.scope) .field("role", &self.role) + .field("scopes", &self.scopes) .field("bearer_token", &"[redacted]") .finish() } @@ -114,6 +120,7 @@ impl Subject { workspace_id: "default".to_string(), scope: Scope::Workspace, role: Role::WorkspaceAdmin, + scopes: Vec::new(), bearer_token: String::new(), } } @@ -168,6 +175,7 @@ mod tests { workspace_id: "default".to_string(), scope, role, + scopes: Vec::new(), bearer_token: "bearer".to_string(), } } @@ -210,6 +218,7 @@ mod tests { workspace_id: "default".to_string(), scope: Scope::Workspace, role: Role::WorkspaceAdmin, + scopes: Vec::new(), bearer_token: String::new(), }; assert!(!s.is_system(), "non-system service account must not match"); diff --git a/crates/alien-manager/src/providers/oss_authz.rs b/crates/alien-manager/src/providers/oss_authz.rs index e6c9b82a..6895992c 100644 --- a/crates/alien-manager/src/providers/oss_authz.rs +++ b/crates/alien-manager/src/providers/oss_authz.rs @@ -210,6 +210,7 @@ mod tests { workspace_id: "default".to_string(), scope: Scope::Workspace, role: Role::WorkspaceAdmin, + scopes: Vec::new(), bearer_token: "bearer".to_string(), } } @@ -225,6 +226,7 @@ mod tests { deployment_group_id: dg_id.to_string(), }, role: Role::DeploymentGroupDeployer, + scopes: Vec::new(), bearer_token: "bearer".to_string(), } } @@ -240,6 +242,7 @@ mod tests { deployment_id: deployment_id.to_string(), }, role: Role::DeploymentManager, + scopes: Vec::new(), bearer_token: "bearer".to_string(), } } diff --git a/crates/alien-manager/src/providers/permissive_auth.rs b/crates/alien-manager/src/providers/permissive_auth.rs index 46bb7a26..1971df61 100644 --- a/crates/alien-manager/src/providers/permissive_auth.rs +++ b/crates/alien-manager/src/providers/permissive_auth.rs @@ -31,6 +31,7 @@ impl AuthValidator for PermissiveAuthValidator { workspace_id: "default".to_string(), scope: Scope::Workspace, role: Role::WorkspaceAdmin, + scopes: Vec::new(), bearer_token: bearer, })) } diff --git a/crates/alien-manager/src/providers/token_db_validator.rs b/crates/alien-manager/src/providers/token_db_validator.rs index e573d65f..abae5274 100644 --- a/crates/alien-manager/src/providers/token_db_validator.rs +++ b/crates/alien-manager/src/providers/token_db_validator.rs @@ -146,6 +146,7 @@ fn record_to_subject(record: &crate::traits::token_store::TokenRecord, bearer: S workspace_id: "default".to_string(), scope, role, + scopes: Vec::new(), bearer_token: bearer, } } diff --git a/crates/alien-manager/src/routes/registry_proxy.rs b/crates/alien-manager/src/routes/registry_proxy.rs index 3e12ce33..3cdd0d93 100644 --- a/crates/alien-manager/src/routes/registry_proxy.rs +++ b/crates/alien-manager/src/routes/registry_proxy.rs @@ -729,15 +729,31 @@ fn require_push_auth(state: &AppState, subject: &Subject, repo_name: &str) -> Re .registry_routing_table .project_id_for_repo(repo_name) .unwrap_or("default"); - if state.authz.can_push_image(subject, project_id, repo_name) { - Ok(()) - } else { - Err(oci_error( + if !state.authz.can_push_image(subject, project_id, repo_name) { + return Err(oci_error( StatusCode::FORBIDDEN, "DENIED", "Caller cannot push images to this project.", - )) + )); } + + // Require at least one JWT scope to cover this project. Suffix-match on + // `/{project_id}` (not the full tuple) because system-manager-minted JWTs + // carry the manager's workspace_id in `subject.workspace_id` but the + // caller's workspace in the scope; project IDs are globally unique so the + // suffix is sufficient. OSS validators leave scopes empty → fall through. + if !subject.scopes.is_empty() { + let suffix = format!("/{}", project_id); + if !subject.scopes.iter().any(|s| s.ends_with(&suffix)) { + return Err(oci_error( + StatusCode::FORBIDDEN, + "DENIED", + "Caller's token scope does not cover this project.", + )); + } + } + + Ok(()) } /// Validate that a deployment token can access the requested repo. diff --git a/crates/alien-manager/tests/registry_proxy_cloud_test.rs b/crates/alien-manager/tests/registry_proxy_cloud_test.rs index db25b5e6..3fc7c69d 100644 --- a/crates/alien-manager/tests/registry_proxy_cloud_test.rs +++ b/crates/alien-manager/tests/registry_proxy_cloud_test.rs @@ -48,6 +48,7 @@ fn test_subject() -> Subject { workspace_id: "default".to_string(), scope: Scope::Workspace, role: Role::WorkspaceAdmin, + scopes: Vec::new(), bearer_token: String::new(), } } diff --git a/crates/alien-manager/tests/registry_proxy_test.rs b/crates/alien-manager/tests/registry_proxy_test.rs index 1d9d8546..2206d461 100644 --- a/crates/alien-manager/tests/registry_proxy_test.rs +++ b/crates/alien-manager/tests/registry_proxy_test.rs @@ -49,6 +49,7 @@ fn test_subject() -> Subject { workspace_id: "default".to_string(), scope: Scope::Workspace, role: Role::WorkspaceAdmin, + scopes: Vec::new(), bearer_token: String::new(), } } diff --git a/crates/alien-manager/tests/sqlite_store_tests.rs b/crates/alien-manager/tests/sqlite_store_tests.rs index ebae75e3..319ae346 100644 --- a/crates/alien-manager/tests/sqlite_store_tests.rs +++ b/crates/alien-manager/tests/sqlite_store_tests.rs @@ -26,6 +26,7 @@ fn test_subject() -> Subject { workspace_id: "default".to_string(), scope: Scope::Workspace, role: Role::WorkspaceAdmin, + scopes: Vec::new(), bearer_token: String::new(), } } From ab16fee15738fe2e29b771faa876706972bd7dbb Mon Sep 17 00:00:00 2001 From: Itamar Zand Date: Sun, 24 May 2026 01:03:39 +0300 Subject: [PATCH 2/6] feat(cli): exchange OAuth JWT for manager-scoped token on release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manager only validates a small set of credential types — long-lived API keys and platform-issued RS256 tokens. The user OAuth JWT from `alien login` isn't one of them, so the CLI's release flow was 401-ing at the first manager call. Fix: at the top of the platform-mode release flow, exchange the user OAuth JWT for a short-lived, project- and-manager-scoped token via the platform's token endpoint, and use that for every subsequent manager call. `mint_registry_push_token` hits `POST {base}/v1/managers/{manager_id}/token` with body `{ purpose: "registry-push", project: }` and the user OAuth bearer. The returned token's scopes/managerId are checked locally on every reuse (`token_matches_target` — globally-unique ULIDs let the project_id check be suffix-only), and a new keyring entry (`MANAGER_TOKEN_USER`) holds the cached token separately from access_token/refresh_token so `logout` cleans both. `get_or_mint_registry_push_token` is the entry point: serve from cache if it's a hit for this target and not within 30s of expiry, otherwise mint fresh and store. Mint failures propagate as `AuthenticationFailed` with the response status + body so platform-side rejection reasons reach the user. In `execution_context.rs`, the platform-mode branch mints once immediately after `resolve` and uses the resulting token both for the `/v1/artifact-registry/*` repo setup and for the `/v2/...` OCI push (both legs of the release flow). `ResolveResponse` now carries `manager_id` so the mint knows which manager the user is targeting. Future platform-mode commands (commands invoke, etc.) should mint their own audience rather than reuse this one — keeps the audience name accurate to scope and prevents a leak of one credential from acting at unintended endpoints. --- crates/alien-cli/src/auth.rs | 158 +++++++++++++++++++++- crates/alien-cli/src/execution_context.rs | 33 ++++- 2 files changed, 183 insertions(+), 8 deletions(-) diff --git a/crates/alien-cli/src/auth.rs b/crates/alien-cli/src/auth.rs index 044ba560..9dca1434 100644 --- a/crates/alien-cli/src/auth.rs +++ b/crates/alien-cli/src/auth.rs @@ -174,6 +174,10 @@ mod oauth_flow { const SERVICE: &str = "alien-cli"; const ACCESS_USER: &str = "access_token"; const REFRESH_USER: &str = "refresh_token"; + /// Manager-scoped Platform JWT minted on `alien login`. Different audience + /// from the user OAuth access_token, so the manager can verify it locally + /// against its configured public key without a /v1/whoami forward. + const MANAGER_TOKEN_USER: &str = "manager_token"; const DEFAULT_BASE: &str = "https://api.alien.dev"; const CLI_CLIENT_ID: &str = "alien-cli"; @@ -280,6 +284,7 @@ mod oauth_flow { pub fn logout() { let _ = Entry::new(SERVICE, ACCESS_USER).and_then(|e| e.delete_password()); let _ = Entry::new(SERVICE, REFRESH_USER).and_then(|e| e.delete_password()); + let _ = Entry::new(SERVICE, MANAGER_TOKEN_USER).and_then(|e| e.delete_password()); let _ = std::fs::remove_file(cfg_path()); with_cache(|cache| cache.clear()); } @@ -472,6 +477,154 @@ mod oauth_flow { Ok(()) } + /// Exchanges a user OAuth JWT for a project-scoped registry-push JWT via + /// `POST {base}/v1/managers/{manager_id}/token`. The result is the only + /// credential the manager accepts on `/v2/...` pushes (single scope tuple, + /// `managerId`-bound, audience `alien:manager:registry-push`). + pub async fn mint_registry_push_token( + client: &Client, + base: &str, + user_jwt: &str, + workspace: &str, + manager_id: &str, + project_id: &str, + ) -> Result { + #[derive(serde::Deserialize)] + struct ExchangeResponse { + #[serde(rename = "accessToken")] + access_token: String, + } + + #[derive(serde::Serialize)] + struct ExchangeBody<'a> { + purpose: &'a str, + project: &'a str, + } + + let response = client + .post(format!("{}/v1/managers/{}/token", base, manager_id)) + .query(&[("workspace", workspace)]) + .header("Authorization", format!("Bearer {}", user_jwt)) + .json(&ExchangeBody { + purpose: "registry-push", + project: project_id, + }) + .send() + .await + .into_alien_error() + .context(ErrorData::AuthenticationFailed { + reason: "Failed to call manager-token exchange endpoint".to_string(), + })?; + + if !response.status().is_success() { + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + return Err(AlienError::new(ErrorData::AuthenticationFailed { + reason: format!( + "manager-token exchange returned status {}: {}", + status, body + ), + })); + } + + let body: ExchangeResponse = response.json().await.into_alien_error().context( + ErrorData::AuthenticationFailed { + reason: "Failed to parse manager-token exchange response".to_string(), + }, + )?; + + Ok(body.access_token) + } + + pub fn store_manager_token(token: &str) -> Result<()> { + Entry::new(SERVICE, MANAGER_TOKEN_USER) + .into_alien_error() + .context(ErrorData::AuthenticationFailed { + reason: "Failed to create manager token keyring entry".to_string(), + })? + .set_password(token) + .into_alien_error() + .context(ErrorData::AuthenticationFailed { + reason: "Failed to store manager token".to_string(), + })?; + Ok(()) + } + + pub fn load_manager_token() -> Option { + Entry::new(SERVICE, MANAGER_TOKEN_USER) + .ok()? + .get_password() + .ok() + } + + /// Checks `managerId` equality and that at least one `scopes` entry ends + /// with `/{project_id}`. Project IDs are globally-unique `prj_*` ULIDs, so + /// the suffix check is equivalent to an exact match on the project segment + /// without needing `workspace_id` (the CLI doesn't have it locally). + /// Returns false on any decode failure — caller re-mints, avoiding a + /// guaranteed 401 on the wire. + fn token_matches_target(jwt: &str, manager_id: &str, project_id: &str) -> bool { + let Some(payload_b64) = jwt.split('.').nth(1) else { + return false; + }; + let Ok(payload_bytes) = URL_SAFE_NO_PAD.decode(payload_b64) else { + return false; + }; + let Ok(claims) = serde_json::from_slice::(&payload_bytes) else { + return false; + }; + let mid_ok = claims + .get("managerId") + .and_then(|v| v.as_str()) + .is_some_and(|m| m == manager_id); + let suffix = format!("/{}", project_id); + let scope_ok = claims + .get("scopes") + .and_then(|v| v.as_array()) + .is_some_and(|arr| { + arr.iter() + .filter_map(|s| s.as_str()) + .any(|s| s.ends_with(&suffix)) + }); + mid_ok && scope_ok + } + + /// Returns a valid registry-push JWT for the (manager, project) target. + /// Reuses the keyring-cached JWT when it matches; common re-mint case is + /// the user switching projects between `alien release` invocations. + pub async fn get_or_mint_registry_push_token( + http: &AuthHttp, + workspace: &str, + manager_id: &str, + project_id: &str, + ) -> Result { + if let Some(existing) = load_manager_token() { + if !token_expired(&existing, 30) + && token_matches_target(&existing, manager_id, project_id) + { + return Ok(existing); + } + } + + let user_jwt = http.bearer_token.as_deref().ok_or_else(|| { + AlienError::new(ErrorData::AuthenticationFailed { + reason: "No user OAuth JWT available to exchange for manager token".to_string(), + }) + })?; + + let minted = mint_registry_push_token( + &http.client, + &http.base_url, + user_jwt, + workspace, + manager_id, + project_id, + ) + .await?; + store_manager_token(&minted)?; + Ok(minted) + } + async fn refresh_token(base: &str, refresh: &str) -> Result { let auth_url = AuthUrl::new(format!("{}/auth/oauth2/authorize", base)) .into_alien_error() @@ -872,4 +1025,7 @@ mod oauth_flow { // Re-export platform-only functions #[cfg(feature = "platform")] -pub use oauth_flow::{force_login, get_auth_http, logout, store_tokens, try_bearer_client}; +pub use oauth_flow::{ + force_login, get_auth_http, get_or_mint_registry_push_token, logout, store_tokens, + try_bearer_client, +}; diff --git a/crates/alien-cli/src/execution_context.rs b/crates/alien-cli/src/execution_context.rs index 22cb6863..1855fcb4 100644 --- a/crates/alien-cli/src/execution_context.rs +++ b/crates/alien-cli/src/execution_context.rs @@ -426,10 +426,31 @@ impl ExecutionMode { info!("Manager: {}", resolved.manager_url); + // The manager rejects the user OAuth JWT directly; exchange it + // for a manager-scoped Platform JWT (audience registry-push, + // managerId-bound) before talking to any manager endpoint. + // + // Today this single token covers BOTH `/v1/artifact-registry/*` + // repo-setup AND the `/v2/...` OCI push — both legs of the + // release flow share an audience because they're prereqs of + // the same user-facing operation. When other platform-mode + // commands (e.g. `alien commands invoke`) come online they + // must mint their OWN audience instead of reusing this one, + // so the audience name keeps reflecting the actual scope. + let manager_jwt = crate::auth::get_or_mint_registry_push_token( + &http, + &workspace, + &resolved.manager_id, + &resolved.project_id, + ) + .await?; + let manager_http_client = + crate::auth::client_with_header(&format!("Bearer {}", manager_jwt))?; + // Now call the manager directly to create/get the artifact registry repo. // The repo logical name is the project ID. let repo_name = create_or_get_artifact_repo( - &http.client, + &manager_http_client, &resolved.manager_url, &resolved.project_id, platform, @@ -438,19 +459,16 @@ impl ExecutionMode { info!("Repository: {}", repo_name); - // Create manager SDK client reusing the authenticated reqwest client - let authenticated_client = http.client.clone(); - let auth_token = http.bearer_token.clone(); let manager_client = ServerSdkClient::new_with_client( &resolved.manager_url, - authenticated_client.clone(), + manager_http_client.clone(), ); Ok(ManagerContext { manager_url: resolved.manager_url, client: manager_client, - http_client: authenticated_client, - auth_token, + http_client: manager_http_client, + auth_token: Some(manager_jwt), repository_name: Some(repo_name), repository_uri: None, }) @@ -554,6 +572,7 @@ async fn check_server_health(port: u16) -> bool { #[derive(serde::Deserialize)] #[serde(rename_all = "camelCase")] struct ResolveResponse { + manager_id: String, manager_url: String, project_id: String, } From e352d5e2ab8352ec50136f7b8d93d708e85bf2d1 Mon Sep 17 00:00:00 2001 From: Itamar Zand Date: Sun, 24 May 2026 11:45:24 +0300 Subject: [PATCH 3/6] test: cover the per-project scope check on the OCI push path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR added two security-critical predicates that were exercised only by code paths the test suite didn't have a way to reach: the manager's push handler skipped the scope check whenever `subject.scopes` was empty (which every test subject was), and the CLI's `token_matches_target` cache-reuse check was never run on a constructed JWT. A regression in either branch — an off-by-one in the suffix match, the scope list flipped to a different shape, the project_id lookup returning the wrong value — would have shipped silently. Extract the scope-check predicate from `require_push_auth` into a pure `scopes_cover_project(scopes, project_id)` helper so it's unit-testable without standing up a 17-field `AppState`. The five new tests cover: empty-scopes fall-through (OSS standalone preserved), matching scope, non-matching scope, one-of-many matching, and the suffix-substring confusion case (`prj_abc` must not accept a scope ending `prj_abcd`). Add `token_matches_target` tests in the CLI: matching, manager-id mismatch, project-id mismatch, missing claim, missing manager-id claim, malformed JWT, and scopes claim of the wrong type. All failure modes must return false so the caller re-mints rather than presenting an unusable token to the manager. No behaviour change in either function. --- crates/alien-cli/src/auth.rs | 94 +++++++++++++++++++ .../src/routes/registry_proxy.rs | 93 +++++++++++++++--- 2 files changed, 173 insertions(+), 14 deletions(-) diff --git a/crates/alien-cli/src/auth.rs b/crates/alien-cli/src/auth.rs index 9dca1434..c0e4a710 100644 --- a/crates/alien-cli/src/auth.rs +++ b/crates/alien-cli/src/auth.rs @@ -1021,6 +1021,100 @@ mod oauth_flow { } } } + + #[cfg(test)] + mod tests { + //! Tests for cache-reuse logic. `token_matches_target` decides whether + //! a cached manager-scoped JWT can be reused for a given (manager, + //! project) target without re-minting. Failure modes: + //! + //! * A token minted for a *different* manager being reused (would + //! cause a managerId-binding rejection at the manager's verifier + //! and an avoidable round-trip — but more importantly, would + //! surface as a confusing 401 rather than triggering a fresh mint). + //! * A token minted for a *different* project being reused (would + //! fail at the scope check on the OCI push path). + //! * A malformed JWT being treated as "matches" (would skip the + //! re-mint and definitely 401 at the manager). + //! + //! All failure cases must return `false` so the caller re-mints. + use super::*; + + const MANAGER_ID: &str = "mgr_xaji0y6dpbhsahxthv3a3zmf8mdd"; + const OTHER_MANAGER_ID: &str = "mgr_other"; + const PROJECT_ID: &str = "prj_nisyo047zb0ourgk7wguij040q1x"; + const OTHER_PROJECT_ID: &str = "prj_other"; + + /// Build a fake JWT (header.payload.sig) where the payload is the + /// given JSON object base64-url-encoded without padding. The header + /// and signature segments are placeholders — `token_matches_target` + /// only ever decodes the payload, so this is enough to exercise the + /// matching logic. + fn fake_jwt(payload: &str) -> String { + format!( + "header.{}.sig", + URL_SAFE_NO_PAD.encode(payload.as_bytes()) + ) + } + + #[test] + fn matches_when_manager_id_and_project_scope_align() { + let jwt = fake_jwt(&format!( + r#"{{"managerId":"{MANAGER_ID}","scopes":["ws_abc/{PROJECT_ID}"]}}"# + )); + assert!(token_matches_target(&jwt, MANAGER_ID, PROJECT_ID)); + } + + #[test] + fn rejects_when_manager_id_mismatches() { + let jwt = fake_jwt(&format!( + r#"{{"managerId":"{OTHER_MANAGER_ID}","scopes":["ws_abc/{PROJECT_ID}"]}}"# + )); + assert!(!token_matches_target(&jwt, MANAGER_ID, PROJECT_ID)); + } + + #[test] + fn rejects_when_project_scope_mismatches() { + let jwt = fake_jwt(&format!( + r#"{{"managerId":"{MANAGER_ID}","scopes":["ws_abc/{OTHER_PROJECT_ID}"]}}"# + )); + assert!(!token_matches_target(&jwt, MANAGER_ID, PROJECT_ID)); + } + + #[test] + fn rejects_when_scopes_claim_is_missing() { + // Defense in depth: even if managerId matches, a token with no + // scope can't push to the target project, so don't reuse it. + let jwt = fake_jwt(&format!(r#"{{"managerId":"{MANAGER_ID}"}}"#)); + assert!(!token_matches_target(&jwt, MANAGER_ID, PROJECT_ID)); + } + + #[test] + fn rejects_when_manager_id_claim_is_missing() { + let jwt = fake_jwt(&format!( + r#"{{"scopes":["ws_abc/{PROJECT_ID}"]}}"# + )); + assert!(!token_matches_target(&jwt, MANAGER_ID, PROJECT_ID)); + } + + #[test] + fn rejects_malformed_jwt() { + // Must NOT panic on garbage — re-minting is the safe response. + assert!(!token_matches_target("not.a.jwt", MANAGER_ID, PROJECT_ID)); + assert!(!token_matches_target("only-one-segment", MANAGER_ID, PROJECT_ID)); + assert!(!token_matches_target("", MANAGER_ID, PROJECT_ID)); + } + + #[test] + fn rejects_jwt_with_non_array_scopes() { + // A buggy producer that serialized `scopes` as a string instead + // of an array must not be treated as a valid cached token. + let jwt = fake_jwt(&format!( + r#"{{"managerId":"{MANAGER_ID}","scopes":"ws_abc/{PROJECT_ID}"}}"# + )); + assert!(!token_matches_target(&jwt, MANAGER_ID, PROJECT_ID)); + } + } } // Re-export platform-only functions diff --git a/crates/alien-manager/src/routes/registry_proxy.rs b/crates/alien-manager/src/routes/registry_proxy.rs index 3cdd0d93..19507450 100644 --- a/crates/alien-manager/src/routes/registry_proxy.rs +++ b/crates/alien-manager/src/routes/registry_proxy.rs @@ -737,25 +737,39 @@ fn require_push_auth(state: &AppState, subject: &Subject, repo_name: &str) -> Re )); } - // Require at least one JWT scope to cover this project. Suffix-match on - // `/{project_id}` (not the full tuple) because system-manager-minted JWTs - // carry the manager's workspace_id in `subject.workspace_id` but the - // caller's workspace in the scope; project IDs are globally unique so the - // suffix is sufficient. OSS validators leave scopes empty → fall through. - if !subject.scopes.is_empty() { - let suffix = format!("/{}", project_id); - if !subject.scopes.iter().any(|s| s.ends_with(&suffix)) { - return Err(oci_error( - StatusCode::FORBIDDEN, - "DENIED", - "Caller's token scope does not cover this project.", - )); - } + if !scopes_cover_project(&subject.scopes, project_id) { + return Err(oci_error( + StatusCode::FORBIDDEN, + "DENIED", + "Caller's token scope does not cover this project.", + )); } Ok(()) } +/// Per-project scope check for the OCI push path. +/// +/// Returns `true` when the caller's token scopes cover `project_id`. An empty +/// `scopes` slice means "no scope claim was attached to this credential" — OSS +/// validators leave it empty, so the check falls through and the role-based +/// gate above is the only authorization required. When scopes ARE present, +/// at least one must end with `/`. +/// +/// Suffix-match on `/{project_id}` (not the full `workspace_id/project_id` +/// tuple) because system-manager-minted JWTs carry the manager's workspace_id +/// in `subject.workspace_id` but the caller's workspace_id in the scope. +/// Project IDs are globally-unique ULIDs so the suffix is unambiguous, and +/// the leading slash prevents a `project_id` that's a substring suffix of +/// another (e.g. `prj_abc` accidentally matching `prj_abcd`). +fn scopes_cover_project(scopes: &[String], project_id: &str) -> bool { + if scopes.is_empty() { + return true; + } + let suffix = format!("/{}", project_id); + scopes.iter().any(|s| s.ends_with(&suffix)) +} + /// Validate that a deployment token can access the requested repo. /// /// Uses the pull validation cache to avoid repeated DB lookups. Workspace- @@ -1173,4 +1187,55 @@ mod tests { None ); } + + // -- scopes_cover_project --------------------------------------------- + // + // Coverage of the OCI push path's per-project scope check. The full + // path (`require_push_auth`) needs `&AppState` which has ~17 fields, + // so the security-critical predicate is extracted into a pure helper + // and tested directly. Failure modes: + // * suffix-substring confusion (e.g. `prj_abc` matching `prj_abcd`) + // * empty-scopes semantics flipped (would block OSS standalone) + // * cross-project scope accepted + + #[test] + fn scopes_cover_project_empty_scopes_pass_through_for_oss_validators() { + // OSS validators (`TokenDbValidator`, `PermissiveAuthValidator`) + // leave `Subject::scopes` empty. The push handler must not reject + // those subjects on the scope axis — the role check above is the + // only gate for OSS standalone deployments. + assert!(scopes_cover_project(&[], "prj_anything")); + } + + #[test] + fn scopes_cover_project_matching_scope_passes() { + let scopes = vec!["ws_main/prj_abc".to_string()]; + assert!(scopes_cover_project(&scopes, "prj_abc")); + } + + #[test] + fn scopes_cover_project_different_project_is_rejected() { + let scopes = vec!["ws_main/prj_other".to_string()]; + assert!(!scopes_cover_project(&scopes, "prj_abc")); + } + + #[test] + fn scopes_cover_project_matches_when_any_of_multiple_scopes_covers() { + let scopes = vec![ + "ws_main/prj_other".to_string(), + "ws_main/prj_abc".to_string(), + "ws_main/prj_third".to_string(), + ]; + assert!(scopes_cover_project(&scopes, "prj_abc")); + } + + #[test] + fn scopes_cover_project_requires_leading_slash_not_just_substring_suffix() { + // Without the leading slash in the suffix this would incorrectly + // pass — `"prj_abcd"` ends with `"prj_abc"` as a substring. The + // `/`-prefix is what makes the suffix a project-segment match + // and not a string-suffix match. + let scopes = vec!["ws_main/prj_abcd".to_string()]; + assert!(!scopes_cover_project(&scopes, "prj_abc")); + } } From c56f79027dce1edd30fe894c48c0ff453c218aa6 Mon Sep 17 00:00:00 2001 From: Itamar Zand Date: Sun, 24 May 2026 14:05:17 +0300 Subject: [PATCH 4/6] refactor(cli): match codebase conventions on URL encoding and cache writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small fixes addressing Greptile review on PR #48, both validated against the codebase's established patterns and `alien/CLAUDE.md`: `mint_registry_push_token`: URL-encode `manager_id` when interpolating it into the request path. Every other path-interpolation in the crate already does this — see `execution_context.rs::resolve_url` (platform, project, workspace), the gcp clients (service-account names, GCS object names), and the aws clients. Manager IDs are ULIDs today and contain no URL-special characters, but encoding here keeps the call site correct-by-construction rather than correct-by-coincidence — a future change to the ID format won't silently mis-route the request. `get_or_mint_registry_push_token`: treat the keyring cache write as best-effort. The mint already succeeded and the caller has a valid token in hand; a failed `set_password` just means we'll re-mint on the next invocation. The previous `?` propagation meant a transient keyring error would abort the whole release even though everything needed for it to succeed was in memory. This matches the worked example in `alien/CLAUDE.md` under "When `warn!` Is Appropriate": > "Use `warn!` only for non-critical issues that don't affect > correctness... Core operation continues and will work correctly, > just slower" No behaviour change in the success path. No new failure modes on the error path beyond what's already there. --- crates/alien-cli/src/auth.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/alien-cli/src/auth.rs b/crates/alien-cli/src/auth.rs index c0e4a710..646a6ed8 100644 --- a/crates/alien-cli/src/auth.rs +++ b/crates/alien-cli/src/auth.rs @@ -501,8 +501,18 @@ mod oauth_flow { project: &'a str, } + // URL-encode the path segment to match the established convention in + // the rest of the crate (see `execution_context.rs::resolve_url` and + // the gcp / aws clients). Manager IDs are ULIDs today and contain no + // URL-special characters, but encoding here keeps the call site + // correct-by-construction rather than correct-by-coincidence — a + // future change to the ID format won't silently mis-route the request. let response = client - .post(format!("{}/v1/managers/{}/token", base, manager_id)) + .post(format!( + "{}/v1/managers/{}/token", + base, + urlencoding::encode(manager_id) + )) .query(&[("workspace", workspace)]) .header("Authorization", format!("Bearer {}", user_jwt)) .json(&ExchangeBody { @@ -621,7 +631,16 @@ mod oauth_flow { project_id, ) .await?; - store_manager_token(&minted)?; + // Cache write is best-effort. The mint succeeded, so we hold a valid + // token in memory and the caller can complete the operation. A failed + // keyring write just means we'll re-mint on the next invocation — + // strictly slower, never wrong. This matches the cache-failure pattern + // documented in `alien/CLAUDE.md` ("When `warn!` Is Appropriate"). + if let Err(e) = store_manager_token(&minted) { + tracing::warn!( + "Failed to cache manager token in keyring (will re-mint next run): {e}" + ); + } Ok(minted) } From ba51807ae6494c55c9b809087435d3b04c862ae9 Mon Sep 17 00:00:00 2001 From: Itamar Zand Date: Sun, 24 May 2026 16:09:45 +0300 Subject: [PATCH 5/6] fix(cli): surface manager response body and unblock commands invoke in platform mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CLI fixes that share an execution_context.rs touch surface but are otherwise independent. execution_context.rs::create_or_get_artifact_repo All three response objects (initial GET, POST, retry GET) were consumed by `.json()` in the success branch only; on non-success the body was dropped. When the manager responded with an authentication failure or another structured error, the user saw "Failed to create artifact repository '...' for platform '...' on manager (HTTP 401)" with no further detail — the actual reason (e.g. "Platform JWT not bound to this manager") lived in the response body that we never read. Read each response's body once via `.text().await.unwrap_or_default()` immediately after capturing the status; parse it as JSON in the success branch, include it verbatim in the error message on failure. Pattern matches `auth.rs::mint_registry_push_token:547` for the same reason. `.unwrap_or_default()` keeps behaviour consistent when the body itself errors — we still get the status, just no detail. commands/commands.rs::commands_task `server_sdk_client()`'s Platform-mode arm returns `Err("server_sdk_client() is not available in platform mode. Use sdk_client() instead.")` because the manager URL + JWT aren't known until the project is resolved. The advice in the error message is wrong — `sdk_client()` returns the platform-API client, not the manager client we actually need to invoke commands. Branch in commands_task: for Platform mode, resolve the project then call `ctx.resolve_manager(...)` to get a ManagerContext whose `client` is authenticated with the minted manager JWT and whose `auth_token` carries that same JWT for the separate CommandsClient call later in `invoke_command`. Standalone/Dev modes keep using `ctx.server_sdk_client()` + `ctx.auth_token()` — those carry the manager URL + API key statically on ExecutionMode and don't need per-project resolution. Unblocks step 5 of the onboarding quickstart (`alien commands invoke --deployment X --command Y`) in Platform mode without changing any of the protocol shapes. --- crates/alien-cli/src/commands/commands.rs | 41 ++++++++++++- crates/alien-cli/src/execution_context.rs | 75 ++++++++++------------- 2 files changed, 71 insertions(+), 45 deletions(-) diff --git a/crates/alien-cli/src/commands/commands.rs b/crates/alien-cli/src/commands/commands.rs index 6752812b..044a21cb 100644 --- a/crates/alien-cli/src/commands/commands.rs +++ b/crates/alien-cli/src/commands/commands.rs @@ -64,8 +64,44 @@ pub enum CommandsAction { /// Execute commands task — works in all execution modes. pub async fn commands_task(args: CommandsArgs, ctx: ExecutionMode) -> Result<()> { - let manager = ctx.server_sdk_client()?; - let manager_url = ctx.manager_url(); + // `server_sdk_client()` errors in Platform mode because the manager + // URL + auth aren't known until the project is resolved. For Platform + // mode we resolve the manager (which mints the per-project JWT and + // builds an authenticated client) before doing anything else. For + // Standalone/Dev the existing code path is unchanged — those modes + // already carry the manager URL + API key statically on ExecutionMode. + let (manager, manager_url, auth_token) = { + #[cfg(feature = "platform")] + if matches!(&ctx, ExecutionMode::Platform { .. }) { + // Platform mode: resolve the project (from --project override or + // local config), then ask the platform API for the manager that + // hosts it. `resolve_manager` returns a ManagerContext whose + // `client` is already authenticated with the minted manager JWT + // and whose `auth_token` carries that same JWT for the separate + // CommandsClient HTTP call below. + let (_, project_link) = ctx.resolve_project(None, true).await?; + let mgr_ctx = ctx + .resolve_manager(&project_link.project_id, "aws") + .await?; + ( + mgr_ctx.client, + mgr_ctx.manager_url, + mgr_ctx.auth_token.unwrap_or_default(), + ) + } else { + ( + ctx.server_sdk_client()?, + ctx.manager_url(), + ctx.auth_token().unwrap_or_default().to_string(), + ) + } + #[cfg(not(feature = "platform"))] + ( + ctx.server_sdk_client()?, + ctx.manager_url(), + ctx.auth_token().unwrap_or_default().to_string(), + ) + }; match args.action { CommandsAction::Invoke { @@ -75,7 +111,6 @@ pub async fn commands_task(args: CommandsArgs, ctx: ExecutionMode) -> Result<()> timeout, } => { let is_dev = ctx.is_dev(); - let auth_token = ctx.auth_token().unwrap_or_default().to_string(); invoke_command( &manager, &manager_url, diff --git a/crates/alien-cli/src/execution_context.rs b/crates/alien-cli/src/execution_context.rs index 1855fcb4..7c7b0105 100644 --- a/crates/alien-cli/src/execution_context.rs +++ b/crates/alien-cli/src/execution_context.rs @@ -595,6 +595,12 @@ async fn create_or_get_artifact_repo( manager_url, project_id, platform ); + // Each response below has the same shape: capture status + body once + // (the body has to be read even on non-success — without it, the manager's + // structured error (e.g. "Platform JWT not bound to this manager") gets + // dropped and the user sees only the HTTP status. Pattern matches + // `auth.rs::mint_registry_push_token` for the same reason. + let get_resp = client .get(&get_url) .send() @@ -607,20 +613,14 @@ async fn create_or_get_artifact_repo( ), url: Some(get_url.clone()), })?; + let get_status = get_resp.status(); + let get_body = get_resp.text().await.unwrap_or_default(); - if get_resp.status().is_success() { - let body: serde_json::Value = - get_resp - .json() - .await - .into_alien_error() - .context(ErrorData::ApiRequestFailed { - message: "Failed to parse artifact repository response".to_string(), - url: Some(get_url.clone()), - })?; - - if let Some(name) = body.get("name").and_then(|v| v.as_str()) { - return Ok(name.to_string()); + if get_status.is_success() { + if let Ok(body) = serde_json::from_str::(&get_body) { + if let Some(name) = body.get("name").and_then(|v| v.as_str()) { + return Ok(name.to_string()); + } } } @@ -643,22 +643,14 @@ async fn create_or_get_artifact_repo( ), url: Some(create_url.clone()), })?; - let create_status = create_resp.status(); + let create_body = create_resp.text().await.unwrap_or_default(); if create_status.is_success() { - let body: serde_json::Value = - create_resp - .json() - .await - .into_alien_error() - .context(ErrorData::ApiRequestFailed { - message: "Failed to parse artifact repository response".to_string(), - url: Some(create_url.clone()), - })?; - - if let Some(name) = body.get("name").and_then(|v| v.as_str()) { - return Ok(name.to_string()); + if let Ok(body) = serde_json::from_str::(&create_body) { + if let Some(name) = body.get("name").and_then(|v| v.as_str()) { + return Ok(name.to_string()); + } } } @@ -676,28 +668,27 @@ async fn create_or_get_artifact_repo( ), url: Some(get_url.clone()), })?; + let get_resp2_status = get_resp2.status(); + let get_resp2_body = get_resp2.text().await.unwrap_or_default(); - if get_resp2.status().is_success() { - let body: serde_json::Value = - get_resp2 - .json() - .await - .into_alien_error() - .context(ErrorData::ApiRequestFailed { - message: "Failed to parse artifact repository response".to_string(), - url: Some(get_url.clone()), - })?; - - if let Some(name) = body.get("name").and_then(|v| v.as_str()) { - return Ok(name.to_string()); + if get_resp2_status.is_success() { + if let Ok(body) = serde_json::from_str::(&get_resp2_body) { + if let Some(name) = body.get("name").and_then(|v| v.as_str()) { + return Ok(name.to_string()); + } } } - // Both create and get failed — report the create error + // Both create and get failed — report the create error WITH the manager's + // response body so the user sees the actual reason (auth failure, bad + // payload, scope mismatch, etc.) instead of just an HTTP status. Err(AlienError::new(ErrorData::ApiRequestFailed { message: format!( - "Failed to create artifact repository '{}' for platform '{}' on manager (HTTP {})", - project_id, platform, create_status + "Failed to create artifact repository '{}' for platform '{}' on manager (HTTP {}): {}", + project_id, + platform, + create_status, + create_body.trim() ), url: Some(create_url), })) From 117704712dad9741c5f98219f39fcb59c4c3a7c3 Mon Sep 17 00:00:00 2001 From: Itamar Zand Date: Sun, 24 May 2026 16:53:27 +0300 Subject: [PATCH 6/6] fix(cli): fail loud on success-status responses with unparseable body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile flagged my earlier A5 refactor of `create_or_get_artifact_repo` for swapping the canonical `.json::().await.into_alien_error().context(...)?` pattern (used by the same file's `resolve_url` block at line 378-381 and by `auth.rs:541`) for a bespoke text-first + best-effort parse shape. The new shape silently fell through to the next step on 2xx-status with an unparseable body — a reverse-proxy or WAF returning an HTML 200 would hide the real diagnostic behind a confusing downstream error. `alien/CLAUDE.md`'s "Fail Fast, Don't Fall Back Silently" section explicitly calls out this pattern as a bad pattern; my refactor was the textbook anti-example. Restore fail-loud semantics on all three response-parse sites (`get_resp`, `create_resp`, `get_resp2`) using `serde_json::from_str(&body).into_alien_error().context(...)?` instead of `if let Ok(body) = ...`. The text-first read pattern stays — it's load-bearing for the final error message at the bottom of the function which includes `create_body` for the manager's structured rejection reason. Valid-JSON-without-`name`-field still falls through (intentional: that's the manager's "no such repo" shape and the next step is the correct response). Pre-existing code (mint helpers, auth flow, scope check) untouched — only the 3 response-parse blocks I introduced in the A5 commit. --- crates/alien-cli/src/execution_context.rs | 63 ++++++++++++++++++----- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/crates/alien-cli/src/execution_context.rs b/crates/alien-cli/src/execution_context.rs index 7c7b0105..304d72cf 100644 --- a/crates/alien-cli/src/execution_context.rs +++ b/crates/alien-cli/src/execution_context.rs @@ -617,10 +617,26 @@ async fn create_or_get_artifact_repo( let get_body = get_resp.text().await.unwrap_or_default(); if get_status.is_success() { - if let Ok(body) = serde_json::from_str::(&get_body) { - if let Some(name) = body.get("name").and_then(|v| v.as_str()) { - return Ok(name.to_string()); - } + // Fail loud on parse error (matches the canonical pattern at + // `execution_context.rs:378-381` and `auth.rs:541`). A reverse-proxy + // or WAF responding with an HTML 200 would otherwise silently fall + // through to CREATE, and the user would see a confusing + // "Failed to create artifact repository" instead of the real + // problem. Valid-JSON-without-`name` still falls through — that's + // the manager's "no such repo" shape and the CREATE below is the + // correct next step. + let body: serde_json::Value = + serde_json::from_str(&get_body) + .into_alien_error() + .context(ErrorData::ApiRequestFailed { + message: format!( + "GET artifact registry returned HTTP {} but the response body wasn't parseable JSON", + get_status + ), + url: Some(get_url.clone()), + })?; + if let Some(name) = body.get("name").and_then(|v| v.as_str()) { + return Ok(name.to_string()); } } @@ -647,10 +663,21 @@ async fn create_or_get_artifact_repo( let create_body = create_resp.text().await.unwrap_or_default(); if create_status.is_success() { - if let Ok(body) = serde_json::from_str::(&create_body) { - if let Some(name) = body.get("name").and_then(|v| v.as_str()) { - return Ok(name.to_string()); - } + // Same fail-loud rationale as the GET above — a CREATE that returned + // 2xx with an unparseable body is the kind of upstream weirdness an + // operator needs to see, not have silently masked by the retry-GET. + let body: serde_json::Value = + serde_json::from_str(&create_body) + .into_alien_error() + .context(ErrorData::ApiRequestFailed { + message: format!( + "POST artifact registry returned HTTP {} but the response body wasn't parseable JSON", + create_status + ), + url: Some(create_url.clone()), + })?; + if let Some(name) = body.get("name").and_then(|v| v.as_str()) { + return Ok(name.to_string()); } } @@ -672,10 +699,22 @@ async fn create_or_get_artifact_repo( let get_resp2_body = get_resp2.text().await.unwrap_or_default(); if get_resp2_status.is_success() { - if let Ok(body) = serde_json::from_str::(&get_resp2_body) { - if let Some(name) = body.get("name").and_then(|v| v.as_str()) { - return Ok(name.to_string()); - } + // Final GET in the retry chain — if this returns 2xx with an + // unparseable body, the final "Failed to create artifact repository" + // error below would otherwise swallow the diagnostic that the + // retry-GET response itself was malformed. Surface it. + let body: serde_json::Value = + serde_json::from_str(&get_resp2_body) + .into_alien_error() + .context(ErrorData::ApiRequestFailed { + message: format!( + "GET artifact registry (retry after CREATE) returned HTTP {} but the response body wasn't parseable JSON", + get_resp2_status + ), + url: Some(get_url.clone()), + })?; + if let Some(name) = body.get("name").and_then(|v| v.as_str()) { + return Ok(name.to_string()); } }