diff --git a/README.md b/README.md index bb59e64..a7d798d 100644 --- a/README.md +++ b/README.md @@ -218,6 +218,12 @@ GitHub-linked releases. Clients use it to skip incompatible versions during unversioned installs and to reject incompatible explicit or locked installs before replacing local package files. +For a GitHub-linked package the requirement is read from `sema.toml` **at each +tag**, so every release records the requirement it was published with and +editing the field does not rewrite past releases. A tag whose `sema.toml` is +missing or unreadable records no requirement; a tag whose `sema.toml` parses but +holds an invalid requirement is reported in the package's sync log. + ### Source Locking A package is either **CLI-uploaded** or **GitHub-linked**, never both. Once a package is linked to a repo, it cannot be published via `sema publish`, and vice versa. diff --git a/src/api/github.rs b/src/api/github.rs index 0dba41e..0c7d2af 100644 --- a/src/api/github.rs +++ b/src/api/github.rs @@ -123,18 +123,14 @@ pub async fn link( let mut imported = 0u32; let mut errors = Vec::new(); + let access = github_sync::RepoAccess { + client: &client, + token: &token, + owner: &owner_name, + repo: &repo, + }; for (tag_name, version) in &tags { - match github_sync::sync_tag( - &state.db, - &owner_name, - &repo, - tag_name, - version, - package_id, - manifest.sema_version_req.as_deref(), - ) - .await - { + match github_sync::sync_tag(&state.db, &access, tag_name, version, package_id).await { Ok(true) => imported += 1, Ok(false) => {} Err(e) => { @@ -210,18 +206,14 @@ pub async fn sync( }; let mut imported = 0u32; + let access = github_sync::RepoAccess { + client: &client, + token: &token, + owner: &owner_name, + repo: &repo, + }; for (tag_name, version) in &tags { - match github_sync::sync_tag( - &state.db, - &owner_name, - &repo, - tag_name, - version, - package_id, - None, - ) - .await - { + match github_sync::sync_tag(&state.db, &access, tag_name, version, package_id).await { Ok(true) => imported += 1, Ok(false) => {} Err(e) => { @@ -339,17 +331,31 @@ pub async fn webhook( let (owner_name, repo) = github_sync::parse_github_url(repo_full_name) .ok_or_else(|| ApiError::bad_request("Invalid repo name"))?; - match github_sync::sync_tag( - &state.db, - &owner_name, - &repo, - tag_name, - &version, - package_id, - None, - ) - .await - { + // The webhook has no authenticated user, so read the repo with an owner's stored + // GitHub token. Without it the tag's sema_version_req cannot be read at all. + let owner_user_id = dal::owners::first_user_id(&state.db, package_id) + .await + .ok() + .flatten() + .ok_or_else(|| ApiError::forbidden("Package has no owner"))?; + let token = + github_sync::get_github_token(&state.db, owner_user_id, &state.config.oauth_token_key) + .await + .ok_or_else(|| { + ApiError::new( + StatusCode::BAD_GATEWAY, + "Package owner has no active GitHub connection", + ) + })?; + let client = reqwest::Client::new(); + let access = github_sync::RepoAccess { + client: &client, + token: &token, + owner: &owner_name, + repo: &repo, + }; + + match github_sync::sync_tag(&state.db, &access, tag_name, &version, package_id).await { Ok(true) => { tracing::info!("Webhook: synced {repo_full_name} tag {tag_name} as {version}"); crate::audit::log( diff --git a/src/dal/owners.rs b/src/dal/owners.rs index f6e4981..8e26278 100644 --- a/src/dal/owners.rs +++ b/src/dal/owners.rs @@ -40,6 +40,26 @@ pub async fn package_id_if_owner( Ok(row.and_then(|r| r.try_get("", "id").ok())) } +/// The lowest owner user id for `package_id`, if the package has an owner. +/// +/// The webhook runs with no authenticated user, so it needs an owner's stored GitHub +/// token to read the repo. Ordering by id keeps the choice stable as owners change. +pub async fn first_user_id( + db: &C, + package_id: i64, +) -> Result, DbErr> { + let row = db + .query_one(crate::db::stmt( + db.get_database_backend(), + r#"SELECT o.user_id FROM owners o + WHERE o.package_id = ? + ORDER BY o.user_id ASC"#, + [package_id.into()], + )) + .await?; + Ok(row.and_then(|r| r.try_get("", "user_id").ok())) +} + /// Usernames of every owner of `package_id`. pub async fn list_usernames( db: &C, diff --git a/src/github_sync.rs b/src/github_sync.rs index 07884ae..916e1bc 100644 --- a/src/github_sync.rs +++ b/src/github_sync.rs @@ -169,19 +169,87 @@ pub async fn list_semver_tags( Ok(tags) } +/// Everything needed to read one GitHub repository. +/// +/// Grouping these keeps the owner, repo, and tag from being transposed at a call site +/// where they are all `&str`. +pub struct RepoAccess<'a> { + pub client: &'a reqwest::Client, + pub token: &'a str, + pub owner: &'a str, + pub repo: &'a str, +} + +/// Read `[package].sema_version_req` from `sema.toml` at one tag. +/// +/// A tag with no readable `sema.toml` declares no requirement — tags older than the +/// manifest are normal, and a fetch failure must not block the import. A manifest that +/// IS readable but holds an invalid requirement is an error, so the maintainer sees it +/// in the sync log instead of the release silently losing its constraint. +async fn fetch_tag_sema_version_req( + access: &RepoAccess<'_>, + tag_name: &str, +) -> Result, String> { + let RepoAccess { + client, + token, + owner, + repo, + } = access; + let url = format!("https://api.github.com/repos/{owner}/{repo}/contents/sema.toml"); + let resp = client + .get(url) + // `.query` percent-encodes, so a tag like `release/1.0` stays one parameter. + .query(&[("ref", tag_name)]) + .header("Authorization", format!("Bearer {token}")) + .header("User-Agent", "sema-pkg") + .header("Accept", "application/vnd.github.raw+json") + .send() + .await; + let Ok(resp) = resp else { + return Ok(None); + }; + if !resp.status().is_success() { + return Ok(None); + } + let Ok(content) = resp.text().await else { + return Ok(None); + }; + tag_sema_version_req(&content, tag_name) +} + +/// Extract and validate `[package].sema_version_req` from one tag's `sema.toml`. +/// +/// An unparsable manifest declares no requirement: `sema.toml` is not required to be +/// valid TOML at every historic tag, and refusing the tag would block the import. A +/// manifest that parses but holds a bad requirement is an error, so the maintainer sees +/// it instead of the release silently losing its constraint. +fn tag_sema_version_req(content: &str, tag_name: &str) -> Result, String> { + let Ok(document) = toml::from_str::(content) else { + return Ok(None); + }; + let Some(value) = document + .get("package") + .and_then(|package| package.get("sema_version_req")) + else { + return Ok(None); + }; + let requirement = value + .as_str() + .ok_or_else(|| format!("{tag_name}: sema_version_req must be a string"))?; + validate_sema_version_req(Some(requirement)).map_err(|error| format!("{tag_name}: {error}")) +} + /// Sync a single tag: store metadata and GitHub tarball URL (no blob download). /// Returns Ok(true) if version was created, Ok(false) if it already existed. pub async fn sync_tag( db: &Db, - owner: &str, - repo: &str, + access: &RepoAccess<'_>, tag_name: &str, version: &semver::Version, package_id: i64, - sema_version_req: Option<&str>, ) -> Result { let version_str = version.to_string(); - let sema_version_req = validate_sema_version_req(sema_version_req)?; // Check if version already exists let exists = crate::dal::versions::exists(db, package_id, &version_str) @@ -192,6 +260,11 @@ pub async fn sync_tag( return Ok(false); } + // Read the requirement from the manifest at this tag, not from the default branch: + // each release states its own, and editing the field must not rewrite past releases. + let sema_version_req = fetch_tag_sema_version_req(access, tag_name).await?; + + let (owner, repo) = (access.owner, access.repo); let tarball_url = format!("https://api.github.com/repos/{owner}/{repo}/tarball/{tag_name}"); crate::dal::versions::create_github_version( @@ -507,6 +580,44 @@ mod manifest_tests { assert_eq!(error, "sema_version_req must be a string"); } + #[test] + fn tag_manifest_reads_the_requirement_at_that_tag() { + let content = "[package]\nname = \"policies\"\nsema_version_req = \" >=1.34.0 \"\n"; + assert_eq!( + tag_sema_version_req(content, "v1.0.0").unwrap(), + Some(">=1.34.0".to_string()) + ); + } + + #[test] + fn tag_manifest_without_a_requirement_is_absent_not_an_error() { + // A tag older than the field, or older than sema.toml itself, must still import. + assert_eq!( + tag_sema_version_req("[package]\nname = \"policies\"\n", "v0.1.0").unwrap(), + None + ); + assert_eq!(tag_sema_version_req("", "v0.1.0").unwrap(), None); + assert_eq!( + tag_sema_version_req("not : valid : toml", "v0.1.0").unwrap(), + None + ); + } + + #[test] + fn tag_manifest_with_a_bad_requirement_names_the_tag() { + let error = tag_sema_version_req( + "[package]\nsema_version_req = \"not a requirement\"\n", + "v1.2.3", + ) + .unwrap_err(); + assert!(error.starts_with("v1.2.3: "), "{error}"); + assert!(error.contains("Invalid sema_version_req"), "{error}"); + + let error = + tag_sema_version_req("[package]\nsema_version_req = 34\n", "v1.2.3").unwrap_err(); + assert_eq!(error, "v1.2.3: sema_version_req must be a string"); + } + #[test] fn validator_treats_blank_as_absent_and_limits_length() { assert_eq!(validate_sema_version_req(Some(" ")).unwrap(), None); diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 977d3ff..55eac2d 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -248,6 +248,81 @@ async fn test_publish_and_get_package() { assert_eq!(body["owners"][0], "publisher"); } +#[tokio::test] +async fn test_sema_version_req_survives_publish_and_is_served_under_that_key() { + // The `sema pkg` client reads versions[].sema_version_req from this response and + // refuses an incompatible install. If the key is renamed, dropped, or left out of + // the query, the client silently stops enforcing, so pin the name and the value. + let (app, _dir) = test_app().await; + let session = register_user(app.clone(), "reqpub", "req@example.com").await; + let token = create_api_token(app.clone(), &session, "req-token").await; + + let meta = serde_json::json!({ + "description": "requirement carrier", + "sema_version_req": " >=1.34.0 ", + }); + let res = publish_package_full( + app.clone(), + &token, + "req-pkg", + "1.0.0", + &gzip(b"tarball"), + &serde_json::to_string(&meta).unwrap(), + ) + .await; + assert_eq!(res.status(), StatusCode::CREATED); + + let res = app + .clone() + .oneshot( + Request::builder() + .uri("/api/v1/packages/req-pkg") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(res.status(), StatusCode::OK); + let body = body_json(res).await; + // Stored normalized (trimmed), served under exactly this key. + assert_eq!(body["versions"][0]["sema_version_req"], ">=1.34.0"); +} + +#[tokio::test] +async fn test_publish_rejects_an_invalid_sema_version_req() { + let (app, _dir) = test_app().await; + let session = register_user(app.clone(), "badreq", "badreq@example.com").await; + let token = create_api_token(app.clone(), &session, "badreq-token").await; + + let meta = serde_json::json!({ + "description": "bad requirement", + "sema_version_req": "not a requirement", + }); + let res = publish_package_full( + app.clone(), + &token, + "badreq-pkg", + "1.0.0", + &gzip(b"tarball"), + &serde_json::to_string(&meta).unwrap(), + ) + .await; + assert_eq!(res.status(), StatusCode::BAD_REQUEST); + + // The rejected publish must not have created the package. + let res = app + .clone() + .oneshot( + Request::builder() + .uri("/api/v1/packages/badreq-pkg") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(res.status(), StatusCode::NOT_FOUND); +} + #[tokio::test] async fn test_publish_duplicate_version() { let (app, _dir) = test_app().await;