diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2af7a9e3e..480b94e5f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -18,7 +18,49 @@ concurrency: cancel-in-progress: false jobs: + git-sources: + name: Git dependencies come from storytold + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Git dependencies come from storytold + run: | + set -euo pipefail + test -f Cargo.lock || { echo "Cargo.lock is missing"; exit 1; } + # Source replacement, patches and path overrides in cargo config never show up + # in Cargo.lock. Parsed rather than grepped: TOML has many spellings of one key. + # Cargo still reads the legacy .cargo/config. + python3 - <<'PY' + import os, sys, tomllib + for path in (".cargo/config", ".cargo/config.toml"): + if not os.path.exists(path): + continue + config = tomllib.load(open(path, "rb")) + replaced = [name for name, source in config.get("source", {}).items() + if isinstance(source, dict) and "replace-with" in source] + if replaced or "patch" in config or "paths" in config: + print(f"{path} replaces, patches or overrides a source") + sys.exit(1) + PY + # Fails when Cargo.lock is out of date with the manifests. + cargo metadata --locked --format-version 1 > /dev/null + python3 - <<'PY' + import sys, tomllib + lock = tomllib.load(open("Cargo.lock", "rb")) + bad = sorted({p["source"] for p in lock.get("package", []) + if p.get("source", "").startswith("git+") + and not p["source"].startswith("git+https://github.com/storytold/")}) + if bad: + print("Git dependencies outside github.com/storytold:", *bad, sep="\n") + sys.exit(1) + PY + release: + needs: git-sources # Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos. uses: storytold/craft-common-actions/.github/workflows/release.yml@main with: diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 6623b6cb8..b2cb88fd2 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -144,7 +144,7 @@ pub fn link_missing(a: &designcraft_doc::Asset, document: Option<&Path>) -> bool let Some(link) = &a.link else { return false }; #[cfg(not(target_arch = "wasm32"))] { - crate::io::document_path(link, document).map_or(true, |path| !crate::io::exists(path)) + !crate::io::document_path(link, document).is_ok_and(crate::io::exists) } #[cfg(target_arch = "wasm32")] { @@ -490,8 +490,18 @@ fn relink_folder(s: &mut Session, p: &Value) -> Result { Some(dir) => Some(dir.clone()), None => a.link.as_deref().and_then(|l| crate::io::document_path(l, document).ok()).and_then(|p| p.parent().map(Path::to_path_buf)), }; - let plain_name = name.to_str().is_some_and(|n| !n.contains(['/', '\\']) && craft_docpath::is_local(n)); - let cand = folder.filter(|_| plain_name).map(|f| f.join(&name)); + let plain_name = name.to_str().filter(|n| !n.contains(['/', '\\'])); + // A name that resolves to another file name (a `file:` URL such as `file:%2Ftmp%2Fx.png`, + // a drive-relative `C:x.png`) or a folder the stored-path rules read differently (`a\b` + // on Unix, `alias/..` through a symlink) could lead outside the folder, so only the name + // itself, directly in the folder as the file system resolves it, counts. The link keeps + // the folder as given. + let cand = folder.zip(plain_name).and_then(|(f, n)| { + let c = crate::io::folder_path(n, &f).ok()?; + let real = |p: &Path| crate::io::canonicalize(p).ok(); + let same_folder = c.parent().and_then(real).is_some_and(|p| real(&f) == Some(p)); + (c.file_name().is_some_and(|c| c == n) && same_folder).then_some(c) + }); if let Some(cand) = cand.filter(|c| crate::io::exists(c)) { todo.push((a.id, cand)); } else { diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 5378a411e..b493edf35 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -21,8 +21,7 @@ use std::path::{Path, PathBuf}; /// refused path is never handed to the file system; callers treat it as a missing file. pub fn document_path(stored: &str, document: Option<&Path>) -> Result { let document = document.map(|d| std::path::absolute(d).unwrap_or_else(|_| d.to_path_buf())); - let path = craft_docpath::resolve(stored, document.as_deref())?; - if path.is_absolute() { Ok(path) } else { Err(Refused::Invalid) } + craft_docpath::resolve_absolute(stored, document.as_deref()) } /// [`document_path`] for a path stored in a file inside `folder` (a data file's image names). diff --git a/crates/engine/src/tests_linked_paths.rs b/crates/engine/src/tests_linked_paths.rs index eb4c953ec..448ffedb9 100644 --- a/crates/engine/src/tests_linked_paths.rs +++ b/crates/engine/src/tests_linked_paths.rs @@ -105,3 +105,73 @@ fn a_relative_link_resolves_beside_the_document() { assert!(seen.iter().all(|p| p.starts_with(&dir)), "{seen:?}"); let _ = std::fs::remove_dir_all(&dir); } + +#[test] +fn a_relative_link_in_an_unsaved_document_is_missing() { + // With no document path there is nothing to resolve against; the working directory is not used. + let mut s = document_with_links(Path::new("doc.designcraft"), &["logo.png", "img/logo.png"]); + s.doc_mut().unwrap().path = None; + let (rows, seen) = crate::io::touched(|| s.execute("links.list", &json!({})).unwrap()); + assert!(seen.is_empty(), "{seen:?}"); + for row in rows.as_array().unwrap() { + assert_eq!(row["status"], "missing", "{row}"); + assert_eq!(row["refused"], "the path is not valid", "{row}"); + } +} + +#[cfg(unix)] +#[test] +fn relink_folder_never_leaves_the_chosen_folder() { + // A stored name with no separator can still be a `file:` URL that decodes to a path elsewhere. + let dir = folder("relink-outside"); + let chosen = dir.join("chosen"); + std::fs::create_dir_all(&chosen).unwrap(); + let outside = dir.join("outside.png"); + std::fs::write(&outside, png()).unwrap(); + let encoded = format!("file:{}", outside.to_string_lossy().replace('/', "%2F")); + let mut s = document_with_links(&dir.join("doc.designcraft"), &[&encoded]); + let (out, seen) = + crate::io::touched(|| s.execute("links.relinkFolder", &json!({"dir": chosen.to_string_lossy(), "missingOnly": false})).unwrap()); + assert_eq!(out["relinked"], 0, "{out}"); + assert!(!seen.contains(&outside), "{seen:?}"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[cfg(unix)] +#[test] +fn relink_folder_takes_the_chosen_folder_as_one_folder() { + // `a\b` is one folder name on Unix; read as a stored path it would be `a/b`. + let dir = folder("relink-backslash"); + let chosen = dir.join("a\\b"); + std::fs::create_dir_all(&chosen).unwrap(); + std::fs::create_dir_all(dir.join("a/b")).unwrap(); + let elsewhere = dir.join("a/b/x.png"); + std::fs::write(&elsewhere, png()).unwrap(); + let mut s = document_with_links(&dir.join("doc.designcraft"), &["x.png"]); + let (out, seen) = + crate::io::touched(|| s.execute("links.relinkFolder", &json!({"dir": chosen.to_string_lossy(), "missingOnly": false})).unwrap()); + assert_eq!(out["relinked"], 0, "{out}"); + assert!(!seen.contains(&elsewhere), "{seen:?}"); + + // A folder given with `..` is still that folder. + let real = dir.join("real"); + std::fs::create_dir_all(&real).unwrap(); + std::fs::write(real.join("x.png"), png()).unwrap(); + let roundabout = real.join("..").join("real"); + let out = s.execute("links.relinkFolder", &json!({"dir": roundabout.to_string_lossy(), "missingOnly": false})).unwrap(); + assert_eq!(out["relinked"], 1, "{out}"); + + // Through a symlink, `alias/..` is the link target's parent, not the folder holding `alias`. + let inner = dir.join("inner"); + std::fs::create_dir_all(inner.join("sub")).unwrap(); + std::os::unix::fs::symlink(inner.join("sub"), dir.join("alias")).unwrap(); + let beside_alias = dir.join("x.png"); + std::fs::write(&beside_alias, png()).unwrap(); + let mut s = document_with_links(&dir.join("doc.designcraft"), &["x.png"]); + let (out, seen) = crate::io::touched(|| { + s.execute("links.relinkFolder", &json!({"dir": dir.join("alias").join("..").to_string_lossy(), "missingOnly": false})).unwrap() + }); + assert_eq!(out["relinked"], 0, "{out}"); + assert!(!seen.contains(&beside_alias), "{seen:?}"); + let _ = std::fs::remove_dir_all(&dir); +}