Skip to content
Open
Show file tree
Hide file tree
Changes from 10 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
52 changes: 47 additions & 5 deletions src/builds.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::auth::AuthManager;
use crate::config::{self, WavedashConfig};
use crate::config::{self, EngineKind, WavedashConfig};
use crate::dev::entrypoint::{fetch_entrypoint_params, resolve_defold_entrypoint};
use crate::file_staging::FileStaging;
use anyhow::Result;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -148,6 +149,15 @@ pub async fn handle_build_push(config_path: PathBuf, verbose: bool, message: Opt
anyhow::bail!("Source must be a directory: {}", upload_dir.display());
}

let engine_kind = wavedash_config.engine_type()?;
let defold_entrypoint = match engine_kind {
Some(EngineKind::Defold) => Some(resolve_defold_entrypoint(
&upload_dir,
wavedash_config.entrypoint.as_deref(),
)?),
_ => None,
};

// Validate required files exist in upload directory
FileStaging::prepare(&upload_dir, &wavedash_config)?;

Expand All @@ -157,14 +167,46 @@ pub async fn handle_build_push(config_path: PathBuf, verbose: bool, message: Opt
anyhow::bail!("No files found in {}", upload_dir.display());
}

let entrypoint = match (&engine_kind, &defold_entrypoint) {
(Some(EngineKind::Defold), Some((_, html_relative_path))) => {
Some(html_relative_path.as_str())
}
_ => wavedash_config.entrypoint(),
};
Comment thread
cursor[bot] marked this conversation as resolved.
let engine_version = wavedash_config.engine_version();
let entrypoint_params = match engine_kind {
Some(EngineKind::Defold) => {
let (html_path, html_relative_path) = defold_entrypoint
.as_ref()
.expect("defold entrypoint resolved");
let ver = engine_version
.ok_or_else(|| anyhow::anyhow!("DEFOLD engine requires a version"))?;
Some(
fetch_entrypoint_params(
EngineKind::Defold.as_label(),
ver,
html_path,
Some(html_relative_path.as_str()),
)
.await?,
)
Comment thread
cloud9c marked this conversation as resolved.
}
Comment on lines +178 to +193
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FileStaging::prepare at line 153 doesn't validate the Defold entrypoint because wavedash_config.entrypoint() returns None for any engine (see config.rs:292-301: the method only returns the entrypoint when no engine is configured). For Custom builds, FileStaging::prepare enforces .html/.htm/.js extension and file existence. For Defold, both checks are skipped, and validation happens later inside resolve_defold_entrypoint (line 60), which only checks is_file() — extension is never validated.

Result: a user who sets entrypoint = "wasm-web/<game>/something.png" (typo or wrong asset) sails through FileStaging::prepare, gets to resolve_defold_entrypoint which passes is_file(), then fetch_entrypoint_params reads a binary file and POSTs it as htmlContent. The server most likely returns a parse error; the user sees an opaque API failure instead of "Entrypoint must be an HTML file".

Two ways to close this:

  1. Move the Defold validation into FileStaging::prepare so it mirrors the Custom path (check extension + existence early, with a clean error message).
  2. Or have resolve_defold_entrypoint itself validate the extension before reading the file.

Either way, the gap is in FileStaging::preparewavedash_config.entrypoint() (the method) silently filters Defold out of validation, while wavedash_config.entrypoint (the field) is what actually drives Defold behavior at lines 166 and dev/mod.rs:135. Two access patterns for the same field that can drift.

Some(EngineKind::JsDos | EngineKind::Ruffle | EngineKind::RenPy) => {
wavedash_config.executable_entrypoint_params()
}
// Explicit (not `_`) so a new EngineKind forces a decision. Godot/Unity
// params are computed server-side.
Some(EngineKind::Godot | EngineKind::Unity) => None,
None => None,
};

// Get temporary R2 credentials (includes build size)
let engine_kind = wavedash_config.engine_type()?;
let creds = get_temp_credentials(
&wavedash_config.game_id,
engine_kind.map(|e| e.as_label()),
wavedash_config.engine_version(),
wavedash_config.entrypoint(),
wavedash_config.executable_entrypoint_params(),
engine_version,
entrypoint,
entrypoint_params,
message.as_deref(),
total_bytes,
&api_key,
Expand Down
16 changes: 15 additions & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,11 @@ pub struct UnitySection {
pub version: String,
}

#[derive(Debug, Deserialize)]
pub struct DefoldSection {
pub version: String,
}

/// Shape for engines whose runtime is fetched as a single executable file
/// (plus an optional loader script). Used by JSDOS, Ruffle, and Ren'Py.
#[derive(Debug, Deserialize)]
Expand All @@ -168,6 +173,9 @@ pub struct WavedashConfig {
#[serde(rename = "unity")]
pub unity: Option<UnitySection>,

#[serde(rename = "defold")]
pub defold: Option<DefoldSection>,

#[serde(rename = "jsdos")]
pub jsdos: Option<ExecutableEngineSection>,

Expand All @@ -182,6 +190,7 @@ pub struct WavedashConfig {
pub enum EngineKind {
Godot,
Unity,
Defold,
JsDos,
Ruffle,
RenPy,
Expand All @@ -192,6 +201,7 @@ impl EngineKind {
match self {
EngineKind::Godot => "GODOT",
EngineKind::Unity => "UNITY",
EngineKind::Defold => "DEFOLD",
EngineKind::JsDos => "JSDOS",
EngineKind::Ruffle => "RUFFLE",
EngineKind::RenPy => "RENPY",
Expand Down Expand Up @@ -236,6 +246,7 @@ impl WavedashConfig {
let engines: Vec<EngineKind> = [
self.godot.is_some().then_some(EngineKind::Godot),
self.unity.is_some().then_some(EngineKind::Unity),
self.defold.is_some().then_some(EngineKind::Defold),
self.jsdos.is_some().then_some(EngineKind::JsDos),
self.ruffle.is_some().then_some(EngineKind::Ruffle),
self.renpy.is_some().then_some(EngineKind::RenPy),
Expand All @@ -248,7 +259,7 @@ impl WavedashConfig {
0 => Ok(None),
1 => Ok(Some(engines[0])),
_ => anyhow::bail!(
"Config must have at most one engine section: [godot], [unity], [jsdos], [ruffle], or [renpy]"
"Config must have at most one engine section: [godot], [unity], [defold], [jsdos], [ruffle], or [renpy]"
),
}
}
Expand All @@ -270,6 +281,9 @@ impl WavedashConfig {
if let Some(unity) = &self.unity {
return Some(&unity.version);
}
if let Some(defold) = &self.defold {
return Some(&defold.version);
}
self.executable_section().map(|s| s.version.as_str())
}

Expand Down
177 changes: 166 additions & 11 deletions src/dev/entrypoint.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::fs;
use std::path::{Path, PathBuf};
use std::path::{Component, Path, PathBuf};

use anyhow::{Context, Result};
use serde::Deserialize;
Expand All @@ -14,6 +14,11 @@ struct EntrypointParamsResponse {
entrypoint_params: Value,
}

/// Locate the HTML entrypoint for engines that ship a single root export
/// (Godot, Unity): a root `index.html`, falling back to the first `.html` found.
/// Defold doesn't use this — it parses the explicit `entrypoint` from
/// wavedash.toml, since its bundles can nest the HTML under `wasm-web`/`js-web`
/// and ship more than one. No "which export is newest" guessing lives here.
pub fn locate_html_entrypoint(upload_dir: &Path) -> Option<PathBuf> {
let default_index = upload_dir.join("index.html");
if default_index.is_file() {
Expand All @@ -37,23 +42,94 @@ pub fn locate_html_entrypoint(upload_dir: &Path) -> Option<PathBuf> {
None
}

pub async fn fetch_entrypoint_params(engine: &str, engine_version: &str, html_path: &Path) -> Result<Value> {
/// Resolve the developer-named Defold entrypoint HTML to its absolute path plus
/// normalized build-relative path. Defold names its entrypoint explicitly (a
/// bundle can ship both `wasm-web/` and `js-web/`), so there's no inference here
/// — a missing or wrong path is a clear error, not a guess.
pub fn resolve_defold_entrypoint(
upload_dir: &Path,
entrypoint: Option<&str>,
) -> Result<(PathBuf, String)> {
let entrypoint = entrypoint.ok_or_else(|| {
anyhow::anyhow!(
"Defold builds need an `entrypoint` in wavedash.toml pointing to your HTML5 export, e.g.\n entrypoint = \"wasm-web/<game>/index.html\"\n(a Defold bundle can contain both wasm-web/ and js-web/ — pick one)"
)
})?;
let relative_path = entrypoint.replace('\\', "/");
let entrypoint_path = Path::new(&relative_path);
if entrypoint_path.is_absolute()
|| entrypoint_path.components().any(|component| {
matches!(
component,
Component::ParentDir | Component::RootDir | Component::Prefix(_)
)
})
{
anyhow::bail!(
"Defold entrypoint `{}` must be a relative path inside upload_dir ({}).",
entrypoint,
upload_dir.display()
);
}

let lower = relative_path.to_ascii_lowercase();
if !lower.ends_with(".html") && !lower.ends_with(".htm") {
anyhow::bail!(
"Defold entrypoint `{}` must be an HTML file (.html/.htm).",
entrypoint
);
}

let html_path = upload_dir.join(&relative_path);
if !html_path.is_file() {
anyhow::bail!(
"Defold entrypoint `{}` not found under {}. Point `entrypoint` in wavedash.toml at your export's index.html.",
entrypoint,
upload_dir.display()
);
}

let canonical_upload_dir = upload_dir
.canonicalize()
.with_context(|| format!("Failed to canonicalize {}", upload_dir.display()))?;
let canonical_html_path = html_path
.canonicalize()
.with_context(|| format!("Failed to canonicalize {}", html_path.display()))?;
if !canonical_html_path.starts_with(&canonical_upload_dir) {
anyhow::bail!(
"Defold entrypoint `{}` resolves outside upload_dir ({}).",
entrypoint,
upload_dir.display()
);
}

Ok((html_path, relative_path))
Comment thread
cursor[bot] marked this conversation as resolved.
}
Comment on lines +49 to +116
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

resolve_defold_entrypoint joins a user-controlled string straight into upload_dir with no normalization or upload-dir containment check — the entrypoint from wavedash.toml can escape the build directory via .. segments or be an absolute path.

Two concrete misconfigurations:

  1. entrypoint = "../../something/index.html" in wavedash.tomlupload_dir.join("../../something/index.html") resolves above upload_dir. If that file happens to exist, is_file() passes; fs::read_to_string(&html_path) reads its content; and html_relative_path = "../../something/index.html" is sent verbatim to the server as htmlPath. The build then references assets via that escape path on the playsite — almost certainly serving a broken bundle.

  2. entrypoint = "/Users/dev/somethingelse/index.html" → Rust's Path::join with an absolute argument replaces the receiver, so upload_dir.join("/Users/dev/…") is just /Users/dev/…. is_file() and read_to_string both succeed on the absolute path; html_relative_path = "/Users/dev/…" leaks the dev's filesystem layout to the server as htmlPath.

The user "owns their own machine" so this isn't a security exploit, but it surfaces typos and copy-paste mistakes as silent wrong-file uploads instead of clear errors. A cheap guard: canonicalize both and check containment:

let html_path = upload_dir.join(&relative_path);
if !html_path.is_file() {
    anyhow::bail!(/* same as today */);
}
// Reject anything that escapes upload_dir.
let canon_html = html_path
    .canonicalize()
    .with_context(|| format!("Failed to canonicalize {}", html_path.display()))?;
let canon_upload = upload_dir
    .canonicalize()
    .with_context(|| format!("Failed to canonicalize {}", upload_dir.display()))?;
if !canon_html.starts_with(&canon_upload) {
    anyhow::bail!(
        "Defold entrypoint `{}` resolves outside upload_dir ({}). The path must be relative to upload_dir and stay inside it.",
        entrypoint,
        upload_dir.display()
    );
}

Bonus: also reject paths whose lowercase extension isn't .html / .htmFileStaging::prepare does this for Custom builds but skips Defold entirely because wavedash_config.entrypoint() returns None for engines.


pub async fn fetch_entrypoint_params(
engine: &str,
engine_version: &str,
html_path: &Path,
html_relative_path: Option<&str>,
) -> Result<Value> {
let html_content = fs::read_to_string(html_path)
.with_context(|| format!("Failed to read {}", html_path.display()))?;
let api_host = config::get("api_host")?;
let endpoint = format!(
"{}/cli/entrypoint-params",
api_host.trim_end_matches('/')
);
let endpoint = format!("{}/cli/entrypoint-params", api_host.trim_end_matches('/'));

let mut body = serde_json::json!({
"engine": engine,
"engineVersion": engine_version,
"htmlContent": html_content,
});
if let Some(html_path) = html_relative_path {
body["htmlPath"] = serde_json::json!(html_path);
}

let client = config::create_http_client()?;
let response = client
.post(&endpoint)
.json(&serde_json::json!({
"engine": engine,
"engineVersion": engine_version,
"htmlContent": html_content,
}))
.json(&body)
.send()
.await
.with_context(|| "Failed to call CLI entrypoint params endpoint")?;
Expand All @@ -70,3 +146,82 @@ pub async fn fetch_entrypoint_params(engine: &str, engine_version: &str, html_pa

Ok(parsed.entrypoint_params)
}

#[cfg(test)]
mod tests {
use super::*;
use std::time::{SystemTime, UNIX_EPOCH};

fn temp_upload_dir(name: &str) -> PathBuf {
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("system clock before unix epoch")
.as_nanos();
std::env::temp_dir().join(format!(
"wavedash-cli-entrypoint-{name}-{}-{unique}",
std::process::id()
))
}

#[test]
fn resolves_defold_entrypoint_inside_upload_dir() {
let upload_dir = temp_upload_dir("inside");
let html_path = upload_dir.join("wasm-web/example/index.html");
fs::create_dir_all(html_path.parent().expect("html parent")).expect("create dirs");
fs::write(&html_path, "<html></html>").expect("write html");

let (resolved, relative) =
resolve_defold_entrypoint(&upload_dir, Some("wasm-web/example/index.html"))
.expect("resolve entrypoint");

assert_eq!(resolved, html_path);
assert_eq!(relative, "wasm-web/example/index.html");

fs::remove_dir_all(upload_dir).expect("cleanup");
}

#[test]
fn normalizes_defold_entrypoint_backslashes() {
let upload_dir = temp_upload_dir("backslashes");
let html_path = upload_dir.join("wasm-web/example/index.html");
fs::create_dir_all(html_path.parent().expect("html parent")).expect("create dirs");
fs::write(&html_path, "<html></html>").expect("write html");

let (resolved, relative) =
resolve_defold_entrypoint(&upload_dir, Some("wasm-web\\example\\index.html"))
.expect("resolve entrypoint");

assert_eq!(resolved, html_path);
assert_eq!(relative, "wasm-web/example/index.html");

fs::remove_dir_all(upload_dir).expect("cleanup");
}

#[test]
fn rejects_defold_entrypoint_escape() {
let upload_dir = temp_upload_dir("escape");
fs::create_dir_all(&upload_dir).expect("create upload dir");

let err = resolve_defold_entrypoint(&upload_dir, Some("../index.html"))
.expect_err("escape should fail");

assert!(err.to_string().contains("relative path inside upload_dir"));

fs::remove_dir_all(upload_dir).expect("cleanup");
}

#[test]
fn rejects_defold_entrypoint_non_html() {
let upload_dir = temp_upload_dir("non-html");
let asset_path = upload_dir.join("wasm-web/example/something.png");
fs::create_dir_all(asset_path.parent().expect("asset parent")).expect("create dirs");
fs::write(&asset_path, "not html").expect("write asset");

let err = resolve_defold_entrypoint(&upload_dir, Some("wasm-web/example/something.png"))
.expect_err("non-html entrypoint should fail");

assert!(err.to_string().contains("HTML file"));

fs::remove_dir_all(upload_dir).expect("cleanup");
}
Comment on lines +175 to +252
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test cleanup leaks temp directories if any assertion before fs::remove_dir_all panics.

Each test ends with fs::remove_dir_all(upload_dir).expect("cleanup") — but if expect_err, assert_eq!, or assert! panics earlier (e.g., if resolve_defold_entrypoint's behavior regresses), the cleanup line never executes and the temp directory remains under /tmp/wavedash-cli-entrypoint-* indefinitely. On a CI loop that re-runs the test suite, this accumulates.

This won't bite production users — only test infrastructure — but it makes it harder to diagnose flaky-test situations on a long-lived runner.

tempfile::TempDir (already a common Rust testing crate) handles RAII cleanup on Drop so panics don't leak. If pulling in another dev-dependency is heavier than wanted, a manual Drop guard struct around PathBuf works too. Minor / quality-of-life only.

}
Loading
Loading