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
7 changes: 7 additions & 0 deletions .changes/unreleased/library-Fixed-20260411-160000.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
component: library
kind: Fixed
body: |-
Validate overlay names reject path separators in release builds

`OverlayName` previously only checked for forward slashes via `debug_assert`, meaning invalid names with path separators (including backslashes) could slip through in release builds. Added `OverlayName::try_new` which returns an error for names containing `/` or `\`, and updated all user-input paths to use it.
time: 2026-04-11T16:00:00.000000000-07:00
51 changes: 44 additions & 7 deletions src/overlay_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use std::fmt;

use anyhow::bail;

/// A normalized overlay name, as stored in `.ccl` file stems.
///
/// This newtype prevents accidental comparison between overlay names
Expand All @@ -14,19 +16,31 @@ use std::fmt;
pub(crate) struct OverlayName(String);

impl OverlayName {
/// Create a new `OverlayName` from a string.
/// Create a new `OverlayName` from a string that is already known to be valid.
///
/// The name must be a simple overlay name (e.g., `"my-overlay"`),
/// not a path like `"org/repo/name"`.
/// Use this only when the name comes from a trusted source (e.g., file stems
/// from directory listing). For user-provided input, use [`try_new`](Self::try_new).
pub(crate) fn new(name: impl Into<String>) -> Self {
let name = name.into();
debug_assert!(
!name.contains('/'),
!name.contains('/') && !name.contains('\\'),
"OverlayName must not contain path separators: {name}"
);
Self(name)
}

/// Create a new `OverlayName` from user-provided input, validating that it
/// contains no path separators.
///
/// Returns an error if the name contains `/` or `\`.
pub(crate) fn try_new(name: impl Into<String>) -> anyhow::Result<Self> {
let name = name.into();
if name.contains('/') || name.contains('\\') {
bail!("Overlay name must not contain path separators: {name}");
}
Ok(Self(name))
}

/// Get the underlying string slice.
pub(crate) fn as_str(&self) -> &str {
&self.0
Expand Down Expand Up @@ -110,9 +124,32 @@ mod tests {
}

#[test]
#[should_panic(expected = "must not contain path separators")]
fn overlay_name_rejects_paths_in_debug() {
let _ = OverlayName::new("org/repo/name");
fn try_new_rejects_forward_slash() {
let result = OverlayName::try_new("org/repo/name");
assert!(result.is_err());
let msg = result.unwrap_err().to_string();
assert!(
msg.contains("path separators"),
"expected path separator error, got: {msg}"
);
}

#[test]
fn try_new_rejects_backslash() {
let result = OverlayName::try_new(r"org\repo\name");
assert!(result.is_err());
let msg = result.unwrap_err().to_string();
assert!(
msg.contains("path separators"),
"expected path separator error, got: {msg}"
);
}

#[test]
fn try_new_accepts_valid_name() {
let result = OverlayName::try_new("my-overlay");
assert!(result.is_ok());
assert_eq!(result.unwrap().as_str(), "my-overlay");
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,7 @@ fn get_fuzzy_suggestions_multi_source(
) -> Vec<String> {
let available: Vec<String> = manager
.list_overlays_for_repo(org, repo)
.unwrap_or_default()
.into_iter()
.map(|n| n.to_string())
.collect();
Expand Down
29 changes: 20 additions & 9 deletions src/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,11 @@ impl SourceManager {
/// List overlay names for a specific org/repo across all sources.
///
/// Returns unique overlay names (deduplicated across sources).
#[must_use]
pub(crate) fn list_overlays_for_repo(&self, org: &str, repo: &str) -> Vec<OverlayName> {
pub(crate) fn list_overlays_for_repo(
&self,
org: &str,
repo: &str,
) -> anyhow::Result<Vec<OverlayName>> {
let mut names = std::collections::HashSet::new();

for ms in &self.sources {
Expand All @@ -319,7 +322,7 @@ impl SourceManager {
}
if let Ok(overlays) = manager.list_overlays_for_repo(org, repo) {
for overlay in overlays {
names.insert(OverlayName::new(overlay.name));
names.insert(OverlayName::try_new(overlay.name)?);
}
}
}
Expand All @@ -329,7 +332,7 @@ impl SourceManager {
if overlay.org.eq_ignore_ascii_case(org)
&& overlay.repo.eq_ignore_ascii_case(repo)
{
names.insert(OverlayName::new(overlay.name));
names.insert(OverlayName::try_new(overlay.name)?);
}
}
}
Expand All @@ -339,7 +342,7 @@ impl SourceManager {

let mut result: Vec<_> = names.into_iter().collect();
result.sort();
result
Ok(result)
}

/// Get the base path for a named source.
Expand Down Expand Up @@ -1360,7 +1363,9 @@ mod tests {
};

// Should find overlays from cloned source, skip uncloned source
let overlays = manager.list_overlays_for_repo("microsoft", "FluidFramework");
let overlays = manager
.list_overlays_for_repo("microsoft", "FluidFramework")
.unwrap();
assert_eq!(overlays.len(), 2);
assert!(overlays.contains(&OverlayName::new("claude-config")));
assert!(overlays.contains(&OverlayName::new("vscode-settings")));
Expand Down Expand Up @@ -1419,7 +1424,9 @@ mod tests {
};

// Should deduplicate across sources
let overlays = manager.list_overlays_for_repo("microsoft", "FluidFramework");
let overlays = manager
.list_overlays_for_repo("microsoft", "FluidFramework")
.unwrap();
assert_eq!(overlays.len(), 1);
assert_eq!(overlays[0], "claude-config");
}
Expand Down Expand Up @@ -1462,7 +1469,9 @@ mod tests {
};

// Different repo should return empty
let overlays = manager.list_overlays_for_repo("google", "chromium");
let overlays = manager
.list_overlays_for_repo("google", "chromium")
.unwrap();
assert!(overlays.is_empty());
}

Expand Down Expand Up @@ -2059,7 +2068,9 @@ mod tests {

let manager = SourceManager::new(sources, Some(repo_root)).unwrap();

let overlays = manager.list_overlays_for_repo("microsoft", "FluidFramework");
let overlays = manager
.list_overlays_for_repo("microsoft", "FluidFramework")
.unwrap();
assert_eq!(overlays.len(), 2);
assert!(overlays.contains(&OverlayName::new("claude-config")));
assert!(overlays.contains(&OverlayName::new("vscode-settings")));
Expand Down
Loading