Add publish command by consideRatio · Pull Request #249 · sensmetry/sysand

consideRatio · 2026-04-01T13:05:57Z

for Index rework overview #210

Summary

This PR introduces a new sysand publish command to upload .kpar artifacts to a sysand package index.

What’s Included

Adds CLI support: sysand publish --index <URL> [PATH]
Resolves [PATH] explicitly, or defaults to output/<name>-<version>.kpar for the current project
Adds core publish flow that uploads multipart payload to <index>/api/v1/upload:
- metadata JSON with:
  - purl (pkg:sysand/<publisher>/<name>@<version>)
  - sha256_digest of the .kpar
- file with .kpar data (application/zip)
Validates publish inputs before upload:
- version must be valid SemVer 2.0
- license must be valid SPDX expression
- publisher/name must follow strict modern ID rules
Normalizes published ID fields (lowercase, spaces -> hyphens; dots in name are preserved)
Adds robust index URL validation:
- only http/https
- rejects query/fragment
- rejects URLs that already include /api/v1/upload
Adds publish-specific auth behavior:
- only bearer-token credentials are used
- credential matching is performed against the resolved upload URL
- clear failures for missing/ambiguous bearer credentials
Maps HTTP errors to user-friendly CLI errors (401/403/404/409 + generic server errors with summarized body)

Docs

Adds docs/src/commands/publish.md
Adds sysand publish entry to docs summary navigation

Tests

Adds dedicated core publish tests (URL handling, validation, error-body summarization)
Adds extensive CLI integration tests for success paths and failure modes
New tests added: 34 total (12 core + 22 CLI)

Dependency/Config Updates

Enables multipart features for reqwest / reqwest-middleware where needed for upload support

andrius-puksta-sensmetry · 2026-04-02T05:14:01Z

Uploads multipart form (purl + file) to /api/v1/upload

Why not use something like Cargo publish protocol?

Reads and validates project metadata from the .kpar:

publisher present and canonicalizable

name canonicalizable

version valid SemVer 2.0

What about license? Should require it to be an SPDX expression. (EDIT: Agree! RESOLVED)

sysand/src/cli.rs

sysand/src/lib.rs

andrius-puksta-sensmetry · 2026-04-02T06:02:55Z

Uploads multipart form (purl + file) to /api/v1/upload

Why not use something like Cargo publish protocol?

Alaternatively, why not be even simpler: pass the PURL in some header (e.g. X-Sysand-publish-PURL) and the KPAR as the application/zip body?

core/src/commands/publish.rs

docs/src/commands/publish.md

sysand/src/commands/publish.rs

Signed-off-by: Erik Sundell <erik.sundell@sensmetry.com> Signed-off-by: Erik Sundell <erik.sundell+2025@sensmetry.com>

consideRatio · 2026-04-09T13:35:37Z

@andrius-puksta-sensmetry I've now updated this PR after rework and more self-review, and I consider it ready to be reviewed again.

Why not use something like Cargo publish protocol?

Alaternatively, why not be even simpler: pass the PURL in some header (e.g. X-Sysand-publish-PURL) and the KPAR as the application/zip body?

We could go for cargo's publish protocol, but there wasn't a clear why / why not for me here so I picked use of multipart form as it felt a bit more straightforward then using crates binary protocol. I think it is good if we can ensure that we can include misc metadata though, so just having a single application/zip POST form with headers seems too rigid. I for example added sha256_digest to the metadata now so that we can verify that the client submitted what was intended to be submitted for example.

Do you want me to switch to Cargos binary protocol to deliver the metadata and file (but with different metadata content)? It also does the trick.

Signed-off-by: Erik Sundell <erik.sundell+2025@sensmetry.com>

Jonas-Puksta-Sensmetry · 2026-04-10T08:54:30Z

core/src/commands/publish.rs

+    #[error(
+        "publisher field `{0}` is invalid for modern project IDs: must be 3-50 characters, use only ASCII letters and numbers, may include single spaces or hyphens between words, and must start and end with a letter or number"
+    )]
+    InvalidPublisher(Box<str>),


Suggested change

InvalidPublisher(Box<str>),

InvalidPublisher(String),

Is there a reason not to just use String here?

To reduce the error size. Box<str> is 16 bytes, String is 24. Since errors are returned from many places and tend to be somewhat large, they dictate the size of the return types.

(to be fair, here specifically this has no effect on error size)

Does it matter to us if the enum type is a few bytes bigger? It seems like a micro optimization to me.

It is a micro optimization. Doesn't hurt though.

Jonas-Puksta-Sensmetry · 2026-04-10T09:02:30Z

core/src/commands/publish.rs

+};
+
+/// Defensive upper bound on kpar file size (100 MiB) to catch unexpected uploads by mistake.
+const MAX_KPAR_PUBLISH_SIZE: u64 = 100 * 1024 * 1024;


Currently Sysand doesn't support jsons in KPARs (I think), but they will be supported. And the sysml model jsons are really big, at least with the current zip compression, and I wouldn't be surprised if even a simple project will be more than 100 MiB.

This should be fine for now though.

Jonas-Puksta-Sensmetry · 2026-04-10T09:21:23Z

core/src/commands/publish.rs

+
+    let info = info.ok_or(PublishError::MissingInfo)?;
+    // Validate that metadata exists; contents are not used during upload.
+    let _meta = meta.ok_or(PublishError::MissingMeta)?;


Suggested change

let _meta = meta.ok_or(PublishError::MissingMeta)?;

_ = meta.ok_or(PublishError::MissingMeta)?;

Jonas-Puksta-Sensmetry · 2026-04-10T09:28:31Z

core/src/commands/publish.rs

+            );
+            Err(PublishError::ServerError {
+                status,
+                body: summarize_error_body(body_bytes),


Do we want to return these error details to the user in all cases?
The answer might be obvious, but I'm not familiar enough with with this to know for myself. Often the "500 internal server error" message is deliberately lacking in details to not accidentally expose sensitive information.

Jonas-Puksta-Sensmetry · 2026-04-10T09:34:54Z

core/src/commands/publish.rs

+/// Must start and end with an alphanumeric character.
+///
+/// Publish-only; if additional surfaces need this, extract to a shared module.
+fn is_valid_field(s: &str, allow_dot: bool) -> bool {


It would be nice to return a more precise error in case it is invalid. It would be better UX for people to immediately know what exactly sysand doesn't like about the name/publisher

Jonas-Puksta-Sensmetry · 2026-04-10T09:47:49Z

core/src/commands/publish.rs

+            (None, Some(detail)) => detail.to_string(),
+            (None, None) => String::new(),
+        };
+        if !message.is_empty() {


What happens if message is empty? The body was a valid json, so I feel like something should be returned from here either way, and not fall through to the non-json body handling

Jonas-Puksta-Sensmetry · 2026-04-10T09:50:20Z

core/src/commands/publish.rs

+fn summarize_error_text(text: &str) -> String {
+    let trimmed = text.trim();
+    if trimmed.is_empty() {
+        return "empty response body".to_string();


Is "empty response body" an error we want to show to the user? Wouldn't it be better to just return a generic description based on the error code (e.g. "Not Found" for 404)?

Jonas-Puksta-Sensmetry · 2026-04-10T09:55:46Z

core/src/commands/publish.rs

+/// Defensive upper bound on kpar file size (100 MiB) to catch unexpected uploads by mistake.
+const MAX_KPAR_PUBLISH_SIZE: u64 = 100 * 1024 * 1024;
+/// Maximum number of characters to include when summarizing an error response body.
+const MAX_ERROR_BODY_CHARS: usize = 1024;


Are there any known cases when the errors would be really long?

andrius-puksta-sensmetry · 2026-04-10T10:41:23Z

Do you want me to switch to Cargos binary protocol to deliver the metadata and file (but with different metadata content)? It also does the trick.

Your choice, I don't have a strong opinion either way. multipart/form-data imposes a slight performance penalty (for generating the field separator string), but is widely supported (which is IMO much more important).

Either way I think it would be better to have separate fields for publisher, name and version. It's also unclear to me if these are required to match the actual KPAR metadata (IMO they should be the same). If yes, combining them into a PURL can be done server side, as publisher/name/version will have to be checked anyway.

andrius-puksta-sensmetry · 2026-04-10T11:06:39Z

core/src/commands/publish.rs

+    log::info!(
+        "{header}{:>12}{header:#} `{name}` {version} to {index}",
+        "Publishing",
+    );


Maybe mention publisher as well?

andrius-puksta-sensmetry · 2026-04-10T11:22:35Z

core/src/commands/publish.rs

+
+    let build_request = move |c: &reqwest_middleware::ClientWithMiddleware| {
+        let file_part = reqwest::multipart::Part::stream(file_bytes.clone())
+            .file_name(file_name.clone())


Why not use Part::file(<path>)? Also, file_name serves no purpose; name will already identify the part.

I see, stream is to clone cheaply.

andrius-puksta-sensmetry · 2026-04-10T11:22:54Z

core/src/commands/publish.rs

+
+        let form = reqwest::multipart::Form::new()
+            .part("metadata", metadata_part)
+            .part("file", file_part);


Suggested change

.part("file", file_part);

.part("kpar", file_part);

More explicit.

andrius-puksta-sensmetry · 2026-04-10T11:30:03Z

core/src/commands/publish.rs

+        });
+    }
+
+    let mut upload_url = index.clone();


Suggested change

let mut upload_url = index.clone();

let mut upload_url = index.to_owned();

andrius-puksta-sensmetry · 2026-04-10T11:31:05Z

core/src/commands/publish.rs

+    {
+        let mut segments = upload_url
+            .path_segments_mut()
+            .expect("http(s) URLs are hierarchical and must support mutable path segments");


No need for error messages, simple unwrap + comment is fine, this is guaranteed to not fail for http(s)

core/src/commands/publish.rs

andrius-puksta-sensmetry · 2026-04-10T11:52:07Z

core/src/commands/publish.rs

+        .path_segments()
+        .expect("http(s) URLs are hierarchical and must support path segments")
+        .collect();
+    if path_segments.ends_with(&UPLOAD_ENDPOINT_SEGMENTS) {


This is too verbose and inefficient. Use .path() and unify UPLOAD_ENDPOINT_SEGMENTS into a single &str.

andrius-puksta-sensmetry · 2026-04-10T11:54:42Z

core/src/commands/publish.rs

+    {
+        let mut segments = upload_url
+            .path_segments_mut()
+            .expect("http(s) URLs are hierarchical and must support mutable path segments");


Suggested change

.expect("http(s) URLs are hierarchical and must support mutable path segments");

.unwrap();

Since this is the same op as above, no need to explain.

andrius-puksta-sensmetry · 2026-04-10T12:06:32Z

core/src/commands/publish.rs

+    #[error("HTTP request failed: {0}")]
+    Http(#[from] reqwest_middleware::Error),


Suggested change

#[error("HTTP request failed: {0}")]

Http(#[from] reqwest_middleware::Error),

#[error("HTTP request failed: {0:#?}")]

Http(#[from] reqwest_middleware::Error),

Due to the way reqwest implements Error, crucial details get lost for reqwest_middleware::Error unless debug print is used.

andrius-puksta-sensmetry · 2026-04-10T12:07:21Z

core/src/commands/publish.rs

+    #[error("failed to read server response body: {0}")]
+    ResponseBody(#[source] reqwest::Error),


Same here

Suggested change

#[error("failed to read server response body: {0}")]

ResponseBody(#[source] reqwest::Error),

#[error("failed to read server response body: {0:#?}")]

ResponseBody(#[source] reqwest::Error),

andrius-puksta-sensmetry · 2026-04-10T12:08:43Z

core/src/commands/publish.rs

+    NotFound(String),
+
+    #[error(
+        "kpar file is unexpectedly large ({size} bytes, limit is {limit} bytes); verify you are publishing the correct file"


Suggested change

"kpar file is unexpectedly large ({size} bytes, limit is {limit} bytes); verify you are publishing the correct file"

"KPAR file is unexpectedly large ({size} bytes, limit is {limit} bytes); verify you are publishing the correct file"

KPAR is an abbreviation

andrius-puksta-sensmetry · 2026-04-10T12:45:15Z

core/src/config/mod.rs

+    // url is declared a String, but would ideally be declared an Url. However,
+    // that would come with challenges as Url provides no Default impl, which
+    // makes it impossible to use #[derive(Default)] on this struct.
    pub url: String,


Semantically doesn't make sense to implement Default for this. It only seems to be useful in a few tests. For convenience can expose function from_url that populates only url field.

consideRatio mentioned this pull request Apr 1, 2026

Index rework overview #210

Open

consideRatio force-pushed the publish branch from d56e015 to 9ef6e96 Compare April 1, 2026 13:42

consideRatio requested a review from andrius-puksta-sensmetry April 1, 2026 13:43

consideRatio marked this pull request as ready for review April 1, 2026 13:51