Skip to content
Open
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cmd/crates/soroban-spec-typescript/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ soroban-spec = { workspace = true }
thiserror = "1.0.32"
serde = "1.0.82"
serde_derive = "1.0.82"
serde_json = "1.0.82"
serde_json = { version = "1.0.82", features = ["preserve_order"] }
sha2 = "0.9.9"
prettyplease = "0.2.4"
include_dir = { version = "0.7.3", features = ["glob"] }
Expand Down
89 changes: 87 additions & 2 deletions cmd/crates/soroban-spec-typescript/src/boilerplate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{
};
use stellar_xdr::curr::ScSpecEntry;

use super::generate;
use super::{generate, validate_npm_package_name};

static PROJECT_DIR: Dir<'_> = include_dir!("$CARGO_MANIFEST_DIR/src/project_template");

Expand Down Expand Up @@ -52,6 +52,14 @@ impl Project {
network_passphrase: Option<&str>,
spec: &[ScSpecEntry],
) -> std::io::Result<()> {
validate_npm_package_name(contract_name).map_err(|e| {
std::io::Error::new(
std::io::ErrorKind::InvalidInput,
format!(
"output directory name '{contract_name}' is not a valid npm package name: {e}"
),
)
})?;
self.replace_placeholder_patterns(contract_name, contract_id, rpc_url, network_passphrase)?;
self.append_index_ts(spec, contract_id, network_passphrase)
}
Expand Down Expand Up @@ -87,7 +95,12 @@ impl Project {
),
];
let root: &Path = self.as_ref();
["package.json", "README.md", "src/index.ts"]

// Handle package.json with proper JSON serialization
self.replace_package_json(root, contract_name)?;

// Handle non-JSON files with string replacement
["README.md", "src/index.ts"]
.into_iter()
.try_for_each(|file_name| {
let file = &root.join(file_name);
Expand All @@ -99,6 +112,33 @@ impl Project {
})
}

fn replace_package_json(&self, root: &Path, contract_name: &str) -> std::io::Result<()> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't use self, so maybe we could extract it as a static function?

let file = root.join("package.json");
let contents = fs::read_to_string(&file)?;
let mut json: serde_json::Value = serde_json::from_str(&contents).map_err(|e| {
std::io::Error::new(
std::io::ErrorKind::InvalidData,
format!("failed to parse package.json template: {e}"),
)
})?;

if let Some(obj) = json.as_object_mut() {
obj.insert(
"name".to_string(),
serde_json::Value::String(contract_name.to_string()),
);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If package.json parses successfully but isn’t a JSON object, this silently skips updating the name field and writes the file back unchanged. Since the template is expected to be an object, it would be safer to return an InvalidData error when json.as_object_mut() is None so template regressions are caught early.

Suggested change
);
);
} else {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
"package.json template must be a JSON object",
));

Copilot uses AI. Check for mistakes.
}

let serialized = serde_json::to_string_pretty(&json).map_err(|e| {
std::io::Error::new(
std::io::ErrorKind::InvalidData,
format!("failed to serialize package.json: {e}"),
)
})?;
// Append trailing newline to match standard formatting
fs::write(&file, format!("{serialized}\n"))
}

fn append_index_ts(
&self,
spec: &[ScSpecEntry],
Expand Down Expand Up @@ -189,6 +229,51 @@ mod test {
println!("Updated Snapshot!");
}

#[test]
fn test_package_json_name_is_set_correctly() {
let temp_dir = TempDir::new().unwrap();
let _project = init(temp_dir.path()).unwrap();
let pkg_json_path = temp_dir.path().join("package.json");
let contents = fs::read_to_string(&pkg_json_path).unwrap();
let json: serde_json::Value = serde_json::from_str(&contents).unwrap();
assert_eq!(json["name"], "test_custom_types");
let obj = json.as_object().unwrap();
let expected_keys = [
"version",
"name",
"type",
"exports",
"typings",
"scripts",
"dependencies",
"devDependencies",
];
for key in obj.keys() {
assert!(
expected_keys.contains(&key.as_str()),
"unexpected key in package.json: {key}"
Comment on lines +251 to +254
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test hard-codes the complete set of top-level keys in package.json and will fail if the template is legitimately extended (e.g., adding license, files, keywords). To reduce brittleness, consider asserting that the expected keys are present (and/or that key count is unchanged) rather than rejecting any new keys.

Suggested change
for key in obj.keys() {
assert!(
expected_keys.contains(&key.as_str()),
"unexpected key in package.json: {key}"
for key in expected_keys {
assert!(
obj.contains_key(key),
"missing expected key in package.json: {key}"

Copilot uses AI. Check for mistakes.
);
}
}

#[test]
fn test_init_rejects_invalid_contract_name() {
let temp_dir = TempDir::new().unwrap();
let p: Project = temp_dir.path().to_path_buf().try_into().unwrap();
let spec = soroban_spec::read::from_wasm(EXAMPLE_WASM).unwrap();
let result = p.init(
r#"foo","optionalDependencies":{"evil":"1"},"z":""#,
Some("CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE"),
Some("https://rpc-futurenet.stellar.org:443"),
Some("Test SDF Future Network ; October 2022"),
&spec,
);
assert!(result.is_err());
let err = result.unwrap_err();
assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput);
assert!(err.to_string().contains("not a valid npm package name"));
}

fn assert_dirs_equal<P: AsRef<Path>>(dir1: P, dir2: P) {
let walker1 = WalkDir::new(&dir1);
let walker2 = WalkDir::new(&dir2);
Expand Down
99 changes: 99 additions & 0 deletions cmd/crates/soroban-spec-typescript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,68 @@ fn sanitize_string(s: &str) -> String {
.replace('\r', "\\r")
}

/// Validate that a string is a valid npm package name.
///
/// Valid names must:
/// - Be non-empty and at most 214 characters
/// - Contain only lowercase alphanumeric characters, hyphens, dots, and underscores
/// - Not start with a dot or underscore
///
/// Scoped names (e.g. `@scope/name`) are also accepted.
pub fn validate_npm_package_name(name: &str) -> Result<(), String> {
if name.is_empty() {
return Err("npm package name must not be empty".to_string());
}
if name.len() > 214 {
return Err(format!(
"npm package name must be at most 214 characters, got {}",
name.len()
));
}

// Handle scoped packages like @scope/name
let name_to_check = if let Some(rest) = name.strip_prefix('@') {
match rest.split_once('/') {
Some((scope, pkg)) => {
if scope.is_empty() || pkg.is_empty() {
return Err(format!(
"scoped npm package name '{name}' must have non-empty scope and package"
));
}
validate_npm_name_segment(scope)?;
pkg
}
None => {
return Err(format!(
"scoped npm package name '{name}' must contain a '/'"
));
}
}
} else {
name
};

validate_npm_name_segment(name_to_check)
}

fn validate_npm_name_segment(segment: &str) -> Result<(), String> {
if segment.starts_with('.') || segment.starts_with('_') {
return Err(format!(
"npm package name segment '{segment}' must not start with '.' or '_'"
));
}
if let Some(c) = segment
.chars()
.find(|c| !matches!(c, 'a'..='z' | '0'..='9' | '-' | '.' | '_'))
{
return Err(format!(
"npm package name segment '{segment}' contains invalid character '{c}'. \
Only lowercase alphanumeric characters, hyphens, dots, and underscores are allowed"
));
}
Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -664,4 +726,41 @@ mod tests {
assert!(!result.contains(DOC_TEST));
assert!(!result.contains(METHOD_TEST));
}

#[test]
fn test_validate_npm_package_name_valid() {
assert!(validate_npm_package_name("my-contract").is_ok());
assert!(validate_npm_package_name("foo.bar").is_ok());
assert!(validate_npm_package_name("a123").is_ok());
assert!(validate_npm_package_name("test_custom_types").is_ok());
assert!(validate_npm_package_name("@scope/my-pkg").is_ok());
}

#[test]
fn test_validate_npm_package_name_invalid() {
// Empty
assert!(validate_npm_package_name("").is_err());
// Leading dot
assert!(validate_npm_package_name(".hidden").is_err());
// Leading underscore
assert!(validate_npm_package_name("_private").is_err());
// Uppercase
assert!(validate_npm_package_name("MyContract").is_err());
// Special characters
assert!(validate_npm_package_name("foo\"bar").is_err());
assert!(validate_npm_package_name("foo bar").is_err());
assert!(validate_npm_package_name("foo{bar}").is_err());
// JSON injection payload
assert!(
validate_npm_package_name(r#"foo","optionalDependencies":{"evil":"1"},"z":""#).is_err()
);
// Too long (215 chars)
assert!(validate_npm_package_name(&"a".repeat(215)).is_err());
// Exactly 214 is ok
assert!(validate_npm_package_name(&"a".repeat(214)).is_ok());
// Bad scoped names
assert!(validate_npm_package_name("@/pkg").is_err());
assert!(validate_npm_package_name("@scope/").is_err());
assert!(validate_npm_package_name("@scope").is_err());
}
}
4 changes: 4 additions & 0 deletions cmd/soroban-cli/src/commands/contract/bindings/typescript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ pub enum Error {
Spec(#[from] spec_tools::Error),
#[error("Failed to get file name from path: {0:?}")]
FailedToGetFileName(PathBuf),
#[error("--output-dir basename is not a valid npm package name: {0}. Use only lowercase alphanumeric characters, hyphens, dots, and underscores")]
InvalidContractName(String),
#[error(transparent)]
WasmOrContract(#[from] contract_spec::Error),
#[error(transparent)]
Expand Down Expand Up @@ -82,6 +84,8 @@ impl Cmd {
let contract_name = &file_name
.to_str()
.ok_or_else(|| Error::NotUtf8(file_name.to_os_string()))?;
soroban_spec_typescript::validate_npm_package_name(contract_name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate_npm_package_name is called here and then again in boilerplate.rs::init(). Not a big deal, but we'll hit it twice.

.map_err(Error::InvalidContractName)?;
Comment on lines +87 to +88
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate_npm_package_name returns an error reason string, but it’s being passed directly into Error::InvalidContractName. As a result, the error message will print the reason where it claims to print the --output-dir basename, and it won’t include the actual invalid basename. Consider changing InvalidContractName to carry both the basename and the validation reason (or format them together) and map the error with a closure so the message is accurate.

Copilot uses AI. Check for mistakes.
let (resolved_address, network) = match source {
contract_spec::Source::Contract {
resolved_address,
Expand Down
Loading