Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
72 changes: 39 additions & 33 deletions src/api/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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(
Expand Down
20 changes: 20 additions & 0 deletions src/dal/owners.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,26 @@ pub async fn package_id_if_owner<C: ConnectionTrait>(
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<C: ConnectionTrait>(
db: &C,
package_id: i64,
) -> Result<Option<i64>, 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<C: ConnectionTrait>(
db: &C,
Expand Down
119 changes: 115 additions & 4 deletions src/github_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<String>, 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<Option<String>, String> {
let Ok(document) = toml::from_str::<toml::Value>(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<bool, String> {
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)
Expand All @@ -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(
Expand Down Expand Up @@ -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);
Expand Down
75 changes: 75 additions & 0 deletions tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down