fix: reject path-like network and environment names

CHANGELOG.md

@@ -8,6 +8,8 @@ air-gapped signing
 
 # Unreleased
 
+fix: reject path-like network and environment names
+
 # v1.6.0
 
 * chore: the macOS `icp` release binaries are now code-signed with DFINITY's Developer ID certificate. This improves the Keychain experience for identities stored in the keyring. macOS used to treat each unsigned release as a different app, so choosing "Always Allow" lasted only until the next upgrade. The signed binary keeps the same identity across releases, so after you allow access once more on the first signed version, it stays allowed through later upgrades. The signed binary is what you get from the GitHub release (including the shell installer) and npm. `brew install icp-cli` from homebrew-core is not signed, because Homebrew builds that formula from source on its own infrastructure. The binary is not notarized, so a release archive downloaded through a browser still gets a Gatekeeper warning; the shell installer and npm are unaffected.

crates/icp-project/src/project.rs

@@ -138,6 +138,16 @@ pub enum ConsolidateManifestError {
     ))]
     InvalidDependencyAlias { alias: String },
 
+    #[snafu(display(
+        "network name '{name}' is invalid: only ASCII letters, digits, '_' and '-' are allowed"
+    ))]
+    InvalidNetworkName { name: String },
+
+    #[snafu(display(
+        "environment name '{name}' is invalid: only ASCII letters, digits, '_' and '-' are allowed"
+    ))]
+    InvalidEnvironmentName { name: String },
+
     #[snafu(display("project declares two dependencies with the same alias '{alias}'"))]
     DuplicateDependencyAlias { alias: String },
 
@@ -318,14 +328,17 @@ fn is_glob(s: &str) -> bool {
     s.contains('*') || s.contains('?') || s.contains('[') || s.contains('{')
 }
 
-/// Whether `name` is a valid canister name or dependency alias: non-empty and
-/// containing only ASCII letters, digits, `_`, or `-`.
+/// Whether `name` is a valid canister name, dependency alias, network name or
+/// environment name: non-empty and containing only ASCII letters, digits, `_`,
+/// or `-`.
 ///
 /// A single strict rule keeps names safe for every purpose they are reused for —
-/// store-key segments, `PUBLIC_CANISTER_ID:<name>` env vars, DNS subdomains, and
-/// archive paths — so no per-site sanitizing is needed. In particular `:` is the
-/// dependency namespace separator, and `.` / `/` would be ambiguous in
-/// subdomains and paths.
+/// store-key segments, `PUBLIC_CANISTER_ID:<name>` env vars, DNS subdomains,
+/// archive paths, and the path components under `.icp` that network and
+/// environment names become — so no per-site sanitizing is needed. In
+/// particular `:` is the dependency namespace separator, `.` / `/` would be
+/// ambiguous in subdomains and paths, and `/`, `..` or an absolute name would
+/// escape `.icp`.
 fn is_valid_name(name: &str) -> bool {
     !name.is_empty()
         && name
@@ -1449,6 +1462,10 @@ pub async fn consolidate_manifest(
             Item::Manifest(ms) => ms.clone(),
         };
 
+        if !is_valid_name(&m.name) {
+            return InvalidNetworkNameSnafu { name: m.name }.fail();
+        }
+
         match networks.entry(m.name.to_owned()) {
             // Duplicate
             Entry::Occupied(e) => {
@@ -1529,6 +1546,10 @@ pub async fn consolidate_manifest(
             Item::Manifest(ms) => ms.clone(),
         };
 
+        if !is_valid_name(&m.name) {
+            return InvalidEnvironmentNameSnafu { name: m.name }.fail();
+        }
+
         match environments.entry(m.name.to_owned()) {
             // Duplicate
             Entry::Occupied(e) => {
@@ -2608,6 +2629,35 @@ environments:
         );
     }
 
+    #[tokio::test]
+    async fn path_like_network_and_environment_names_are_rejected() {
+        // Both names become path components under `.icp` (the network's state
+        // directory, the environment's `<env>.ids.json`), so anything that could
+        // leave that directory must be refused before it is ever joined.
+        for bad in ["/tmp/evil", "../../evil", "a/b", "..", "a.b", ""] {
+            let tmp = Utf8TempDir::new().unwrap();
+            let body = manifest(
+                &[],
+                &format!("networks:\n  - name: \"{bad}\"\n    mode: managed\n"),
+            );
+            write(tmp.path(), "icp.yaml", &body);
+            let err = consolidate(tmp.path()).await.unwrap_err();
+            assert!(
+                matches!(err, ConsolidateManifestError::InvalidNetworkName { .. }),
+                "network {bad:?}: got {err:?}"
+            );
+
+            let tmp = Utf8TempDir::new().unwrap();
+            let body = manifest(&[], &format!("environments:\n  - name: \"{bad}\"\n"));
+            write(tmp.path(), "icp.yaml", &body);
+            let err = consolidate(tmp.path()).await.unwrap_err();
+            assert!(
+                matches!(err, ConsolidateManifestError::InvalidEnvironmentName { .. }),
+                "environment {bad:?}: got {err:?}"
+            );
+        }
+    }
+
     #[tokio::test]
     async fn dot_in_canister_name_is_rejected() {
         let tmp = Utf8TempDir::new().unwrap();