fix: require complete braced environment references

crates/guest-contract/src/instance_env.rs

@@ -1,6 +1,6 @@
 use protocol::{
-    AppHostname, AppHostnameKind, GuestPath, Hostname, HttpPort, RestartPolicy, TenantArguments,
-    TenantEnvironment,
+    runtime_references, AppHostname, AppHostnameKind, GuestPath, Hostname, HttpPort, RestartPolicy,
+    TenantArguments, TenantEnvironment, RUNTIME_VALUE_NAMES,
 };
 
 pub const INSTANCE_ENV_FILENAME: &str = "instance.env";
@@ -132,50 +132,23 @@ pub struct InstanceConfig {
 }
 
 fn expand(key: &str, value: &str, runtime: &[(String, String)]) -> Result<String, InstanceEnvError> {
-    if !value.contains(&format!("${RUNTIME_PREFIX}")) && !value.contains(&format!("${{{RUNTIME_PREFIX}")) {
-        return Ok(value.to_string());
-    }
     let mut expanded = String::with_capacity(value.len());
-    let bytes = value.as_bytes();
     let mut at = 0;
-    while at < bytes.len() {
-        if bytes[at] != b'$' {
-            expanded.push(value[at..].chars().next().unwrap_or('$'));
-            at += value[at..].chars().next().map_or(1, char::len_utf8);
-            continue;
-        }
-        let braced = value[at + 1..].starts_with('{');
-        let names_at = at + 1 + usize::from(braced);
-        let rest = &value[names_at..];
-        if !rest.starts_with(RUNTIME_PREFIX) {
-            expanded.push('$');
-            at += 1;
-            continue;
-        }
-        let end = if braced {
-            match rest.find('}') {
-                Some(end) => end,
-                None => {
-                    return Err(InstanceEnvError::Malformed {
-                        key: key.to_string(),
-                        rule: "a closed reference",
-                    })
-                }
-            }
-        } else {
-            rest.find(|c: char| !c.is_ascii_alphanumeric() && c != '_')
-                .unwrap_or(rest.len())
-        };
-        let reference = &rest[..end];
-        let Some((_, found)) = runtime.iter().find(|(name, _)| name == reference) else {
+    for (range, reference) in runtime_references(value) {
+        let found = runtime
+            .iter()
+            .find(|(name, _)| name == reference && RUNTIME_VALUE_NAMES.contains(&reference));
+        let Some((_, found)) = found else {
             return Err(InstanceEnvError::UnknownReference {
                 key: key.to_string(),
                 reference: reference.to_string(),
             });
         };
+        expanded.push_str(&value[at..range.start]);
         expanded.push_str(found);
-        at = names_at + end + usize::from(braced);
+        at = range.end;
     }
+    expanded.push_str(&value[at..]);
     Ok(expanded)
 }
 
@@ -559,13 +532,14 @@ mod both_ends {
     }
 
     #[test]
-    fn only_a_reference_to_a_runtime_value_expands() {
+    fn only_a_complete_reference_to_a_runtime_value_expands() {
         let config = parse_instance_env(&written(
             &[
                 ("BARE", "port $NIBRUN_HTTP_PORT here"),
                 ("BRACED", "port ${NIBRUN_HTTP_PORT} here"),
                 ("SHELL_LIKE", "$HOME and $$ and a bare $"),
-                ("BOTH", "$NIBRUN_HOSTNAME:$NIBRUN_HTTP_PORT"),
+                ("BOTH", "${NIBRUN_HOSTNAME}:${NIBRUN_HTTP_PORT}"),
+                ("TWICE", "${NIBRUN_HTTP_PORT}${NIBRUN_HTTP_PORT}0"),
             ],
             &[],
         ))
@@ -579,26 +553,97 @@ mod both_ends {
                 .unwrap()
         };
         let port = DEFAULT_HTTP_PORT.to_string();
-        assert_eq!(value("BARE"), format!("port {port} here"));
+        assert_eq!(value("BARE"), "port $NIBRUN_HTTP_PORT here");
         assert_eq!(value("BRACED"), format!("port {port} here"));
         assert_eq!(value("SHELL_LIKE"), "$HOME and $$ and a bare $");
         assert_eq!(value("BOTH"), format!("my-app.nibrun.app:{port}"));
+        assert_eq!(value("TWICE"), format!("{port}{port}0"));
     }
 
     #[test]
-    fn a_reference_this_runtime_does_not_offer_fails_the_boot_at_both_ends() {
-        assert!(
-            TenantValue::parse("$NIBRUN_NO_SUCH_THING").is_err(),
-            "the host would have written it"
-        );
+    fn secrets_with_bare_names_and_incomplete_references_reach_the_app_unchanged() {
+        for literal in [
+            "$",
+            "{",
+            "}",
+            "${",
+            "${}",
+            "$NIBRUN_HTTP_PORT",
+            "$NIBRUN_NOPE",
+            "$NIBRUN_PUBLIC_IPV4",
+            "$NIBRUN_EXTRA_PUBLIC_PORT",
+            "${NIBRUN_HTTP_PORT",
+            "${NIBRUN_NOPE",
+            "{NIBRUN_HTTP_PORT}",
+            "${NIBRUN_HTTP_PORT!}",
+            "${NIBRUN_HTTP_PORT with spaces}",
+            "secret$NIBRUN_HTTP_PORT}suffix",
+            "$2y$10$K3JqBQ8Rt7uVwXyZaBcDeF",
+            "🔑${NIBRUN_HTTP_PORTé}尾",
+        ] {
+            let config = parse_instance_env(&written(&[("SECRET", literal)], &[])).unwrap();
+            assert!(config
+                .tenant_environment()
+                .contains(&("SECRET".to_string(), literal.to_string())));
+        }
+    }
 
-        let handed = format!("{}ENV_BAD=$NIBRUN_NO_SUCH_THING\n", written(&[], &[]));
-        let error = parse_instance_env(&handed).unwrap_err();
-        assert!(
-            matches!(&error, InstanceEnvError::UnknownReference { reference, .. } if reference == "NIBRUN_NO_SUCH_THING"),
-            "{error}"
+    #[test]
+    fn complete_references_expand_among_literal_characters() {
+        let config = parse_instance_env(&written(
+            &[(
+                "MIXED",
+                "🔑$${NIBRUN_HTTP_PORT}|{${NIBRUN_HOSTNAME}}|${NIBRUN_BROKEN:${NIBRUN_HTTP_PORT}|$尾",
+            )],
+            &[],
+        ))
+        .unwrap();
+        assert_eq!(
+            config.environment,
+            vec![(
+                "MIXED".to_string(),
+                format!(
+                    "🔑${DEFAULT_HTTP_PORT}|{{my-app.nibrun.app}}|${{NIBRUN_BROKEN:{DEFAULT_HTTP_PORT}|$尾"
+                ),
+            )]
         );
-        assert!(error.to_string().contains("BAD"), "{error}");
+    }
+
+    #[test]
+    fn a_reference_this_runtime_does_not_offer_fails_the_boot_at_both_ends() {
+        for (value, name) in [
+            ("${NIBRUN_NO_SUCH_THING}", "NIBRUN_NO_SUCH_THING"),
+            ("${NIBRUN_HTTP_PORT0}", "NIBRUN_HTTP_PORT0"),
+            ("${NIBRUN_MAX_RESTARTS}", "NIBRUN_MAX_RESTARTS"),
+            ("${NIBRUN_}", "NIBRUN_"),
+            ("${NIBRUN_BROKEN:${NIBRUN_NO_SUCH_THING}", "NIBRUN_NO_SUCH_THING"),
+        ] {
+            assert!(
+                TenantValue::parse(value).is_err(),
+                "the host would have written it"
+            );
+
+            let handed = format!("{}ENV_BAD={value}\n", written(&[], &[]));
+            let error = parse_instance_env(&handed).unwrap_err();
+            assert!(
+                matches!(&error, InstanceEnvError::UnknownReference { reference, .. } if reference == name),
+                "{error}"
+            );
+            assert!(error.to_string().contains("BAD"), "{error}");
+        }
+    }
+
+    #[test]
+    fn a_complete_reference_to_a_missing_hostname_fails_the_boot() {
+        let handed = written(&[("URL", "https://${NIBRUN_HOSTNAME}")], &[])
+            .lines()
+            .filter(|line| !line.starts_with("NIBRUN_HOSTNAME="))
+            .collect::<Vec<_>>()
+            .join("\n");
+        assert!(matches!(
+            parse_instance_env(&handed),
+            Err(InstanceEnvError::UnknownReference { reference, .. }) if reference == "NIBRUN_HOSTNAME"
+        ));
     }
 
     #[test]

crates/protocol/schema/desired.schema.json

@@ -774,7 +774,7 @@
       }
     },
     "TenantValue": {
-      "description": "Handed to the app as is, except that `${NAME}` and `$NAME` are filled in for NAME in NIBRUN_HOSTNAME, NIBRUN_HTTP_PORT; any other `$NIBRUN_` reference is refused.",
+      "description": "Only complete `${NAME}` references are filled in for NAME in NIBRUN_HOSTNAME, NIBRUN_HTTP_PORT. Bare names, incomplete references and other dollar signs remain literal; complete references to unknown `NIBRUN_` names are refused.",
       "type": "string",
       "maxLength": 32768
     },

crates/protocol/src/domain.rs

@@ -1,6 +1,7 @@
 #[cfg(feature = "schema")]
 use std::borrow::Cow;
 use std::collections::BTreeMap;
+use std::ops::Range;
 
 #[cfg(feature = "schema")]
 use schemars::{JsonSchema, Schema, SchemaGenerator};
@@ -26,46 +27,22 @@ fn is_name_character(c: char) -> bool {
     c.is_ascii_alphanumeric() || c == '_'
 }
 
-fn runtime_references(value: &str) -> Vec<(String, bool)> {
-    let mut found = Vec::new();
-    let bytes = value.as_bytes();
-    let mut index = 0;
-    while index < bytes.len() {
-        if bytes[index] != b'$' {
-            index += 1;
-            continue;
+pub fn runtime_references(value: &str) -> impl Iterator<Item = (Range<usize>, &str)> {
+    value.match_indices("${").filter_map(|(start, opening)| {
+        let rest = &value[start + opening.len()..];
+        if !rest.starts_with(RUNTIME_VALUE_PREFIX) {
+            return None;
         }
-        let mut cursor = index + 1;
-        let braced = bytes.get(cursor) == Some(&b'{');
-        if braced {
-            cursor += 1;
+        let end = rest.find(|c| !is_name_character(c))?;
+        if rest.as_bytes()[end] != b'}' {
+            return None;
         }
-        if !value[cursor..].starts_with(RUNTIME_VALUE_PREFIX) {
-            index += 1;
-            continue;
-        }
-        let start = cursor;
-        while cursor < bytes.len() && is_name_character(bytes[cursor] as char) {
-            cursor += 1;
-        }
-        let name = &value[start..cursor];
-        let closed = if braced {
-            let closed = bytes.get(cursor) == Some(&b'}');
-            if closed {
-                cursor += 1;
-            }
-            closed
-        } else {
-            true
-        };
-        found.push((name.to_string(), closed && RUNTIME_VALUE_NAMES.contains(&name)));
-        index = cursor.max(index + 1);
-    }
-    found
+        Some((start..start + opening.len() + end + 1, &rest[..end]))
+    })
 }
 
 pub fn names_offered_runtime_values(value: &str) -> bool {
-    runtime_references(value).iter().all(|(_, allowed)| *allowed)
+    runtime_references(value).all(|(_, name)| RUNTIME_VALUE_NAMES.contains(&name))
 }
 
 #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
@@ -122,8 +99,9 @@ impl JsonSchema for TenantValue {
         schema.insert(
             "description".into(),
             format!(
-                "Handed to the app as is, except that `${{NAME}}` and `$NAME` are filled in for NAME \
-                 in {}; any other `$NIBRUN_` reference is refused.",
+                "Only complete `${{NAME}}` references are filled in for NAME in {}. \
+                 Bare names, incomplete references and other dollar signs remain literal; \
+                 complete references to unknown `NIBRUN_` names are refused.",
                 RUNTIME_VALUE_NAMES.join(", ")
             )
             .into(),

crates/protocol/src/tests.rs

@@ -127,10 +127,38 @@ fn a_secret_never_prints_itself() {
 fn a_tenant_value_may_name_only_offered_runtime_values() {
     assert!(TenantValue::parse("$HOME and $$ and a bcrypt $2b$10$abc").is_ok());
     assert!(TenantValue::parse("http://x:${NIBRUN_HTTP_PORT}/").is_ok());
-    assert!(TenantValue::parse("$NIBRUN_HTTP_PORT").is_ok());
-    assert!(TenantValue::parse("$NIBRUN_HTTP_PORTS").is_err());
+    assert!(TenantValue::parse("${NIBRUN_HTTP_PORTS}").is_err());
     assert!(TenantValue::parse("${NIBRUN_NOPE}").is_err());
-    assert!(TenantValue::parse("${NIBRUN_HTTP_PORT").is_err());
+    assert!(TenantValue::parse("${NIBRUN_}").is_err());
+    assert!(TenantValue::parse("${NIBRUN_BROKEN:${NIBRUN_NOPE}").is_err());
+}
+
+#[test]
+fn a_tenant_value_preserves_everything_except_complete_runtime_references() {
+    for literal in [
+        "$",
+        "{",
+        "}",
+        "${",
+        "${}",
+        "$NIBRUN_HTTP_PORT",
+        "$NIBRUN_NOPE",
+        "${NIBRUN_HTTP_PORT",
+        "${NIBRUN_NOPE",
+        "{NIBRUN_HTTP_PORT}",
+        "${NIBRUN_HTTP_PORT!}",
+        "${NIBRUN_HTTP_PORT with spaces}",
+        "secret$NIBRUN_HTTP_PORT}suffix",
+        "🔑${NIBRUN_HTTP_PORTé}尾",
+    ] {
+        assert_eq!(TenantValue::parse(literal).unwrap().expose(), literal);
+        let decoded: TenantValue = serde_json::from_value(serde_json::json!(literal)).unwrap();
+        assert_eq!(decoded.expose(), literal);
+    }
+    assert!(TenantValue::parse(
+        "$${NIBRUN_HTTP_PORT}|{${NIBRUN_HOSTNAME}}|${NIBRUN_BROKEN:${NIBRUN_HTTP_PORT}|$"
+    )
+    .is_ok());
 }
 
 #[test]